Commit 5bf5e2b5 authored by Bryan Thrall's avatar Bryan Thrall
Browse files

Bug 1839007 - Return bool from ErrorToException r=arai a=RyanVM 

It is simpler to think about ErrorToException returning true when the error is
correctly converted, which is why it returns false on recursion even though you
could argue that nothing went wrong in that case.

Differential Revision: https://phabricator.services.mozilla.com/D184160
parent 649d222a

js/src/jsexn.cpp

+8 −7
Original line number Diff line number Diff line
@@ -295,7 +295,7 @@ JS_PUBLIC_API JSLinearString* js::GetErrorTypeName(JSContext* cx, 
  return ClassName(key, cx);

}



void js::ErrorToException(JSContext* cx, JSErrorReport* reportp,

bool js::ErrorToException(JSContext* cx, JSErrorReport* reportp,

                          JSErrorCallback callback, void* userRef) {

  MOZ_ASSERT(!reportp->isWarning());
@@ -311,7 +311,7 @@ void js::ErrorToException(JSContext* cx, JSErrorReport* reportp, 


  // Prevent infinite recursion.

  if (cx->generatingError) {

    return;

    return false;

  }



  cx->generatingError = true;
@@ -320,7 +320,7 @@ void js::ErrorToException(JSContext* cx, JSErrorReport* reportp, 
  // Create an exception object.

  RootedString messageStr(cx, reportp->newMessageString(cx));

  if (!messageStr) {

    return;

    return false;

  }



  Rooted<JSString*> fileName(cx);
@@ -328,7 +328,7 @@ void js::ErrorToException(JSContext* cx, JSErrorReport* reportp, 
    fileName =

        JS_NewStringCopyUTF8N(cx, JS::UTF8Chars(filename, strlen(filename)));

    if (!fileName) {

      return;

      return false;

    }

  } else {

    fileName = cx->emptyString();
@@ -343,19 +343,19 @@ void js::ErrorToException(JSContext* cx, JSErrorReport* reportp, 


  RootedObject stack(cx);

  if (!CaptureStack(cx, &stack)) {

    return;

    return false;

  }



  UniquePtr<JSErrorReport> report = CopyErrorReport(cx, reportp);

  if (!report) {

    return;

    return false;

  }



  ErrorObject* errObject =

      ErrorObject::create(cx, exnType, stack, fileName, sourceId, lineNumber,

                          columnNumber, std::move(report), messageStr, cause);

  if (!errObject) {

    return;

    return false;

  }



  // Throw it.
@@ -365,6 +365,7 @@ void js::ErrorToException(JSContext* cx, JSErrorReport* reportp, 
    nstack = &stack->as<SavedFrame>();

  }

  cx->setPendingException(errValue, nstack);

  return true;

}



using SniffingBehavior = JS::ErrorReportBuilder::SniffingBehavior;

js/src/jsexn.h

+4 −3
Original line number Diff line number Diff line
@@ -50,11 +50,12 @@ JSString* ComputeStackString(JSContext* cx); 
 * The original error described by reportp typically won't be reported anywhere

 * in this case.

 *

 * If the error code is unrecognized, or if we decided to do nothing in order to

 * avoid recursion, we simply return and this error is just being swept under

 * Returns true if the error was converted to an exception. If the error code

 * is unrecognized, we fail due to OOM, or if we decided to do nothing in order

 * to avoid recursion, we return false and this error is just being swept under

 * the rug.

 */

extern void ErrorToException(JSContext* cx, JSErrorReport* reportp,

extern bool ErrorToException(JSContext* cx, JSErrorReport* reportp,

                             JSErrorCallback callback, void* userRef);



extern JSErrorReport* ErrorFromException(JSContext* cx, HandleObject obj);

js/src/vm/ErrorReporting.cpp

+12 −6
Original line number Diff line number Diff line
@@ -137,16 +137,16 @@ void js::ReportErrorToGlobal(JSContext* cx, Handle<GlobalObject*> global, 
  PrepareScriptEnvironmentAndInvoke(cx, global, report);

}



static void ReportError(JSContext* cx, JSErrorReport* reportp,

static bool ReportError(JSContext* cx, JSErrorReport* reportp,

                        JSErrorCallback callback, void* userRef) {

  if (reportp->isWarning()) {

    CallWarningReporter(cx, reportp);

    return;

    return true;

  }



  // Check the error report, and set a JavaScript-catchable exception

  // if the error is defined to have an associated exception.

  ErrorToException(cx, reportp, callback, userRef);

  return ErrorToException(cx, reportp, callback, userRef);

}



/*
@@ -460,7 +460,9 @@ bool js::ReportErrorNumberVA(JSContext* cx, IsWarning isWarning, 
    return false;

  }



  ReportError(cx, &report, callback, userRef);

  if (!ReportError(cx, &report, callback, userRef)) {

    return false;

  }



  return report.isWarning();

}
@@ -501,7 +503,9 @@ static bool ReportErrorNumberArray(JSContext* cx, IsWarning isWarning, 
    return false;

  }



  ReportError(cx, &report, callback, userRef);

  if (!ReportError(cx, &report, callback, userRef)) {

    return false;

  }



  return report.isWarning();

}
@@ -550,7 +554,9 @@ bool js::ReportErrorVA(JSContext* cx, IsWarning isWarning, const char* format, 
  }

  PopulateReportBlame(cx, &report);



  ReportError(cx, &report, nullptr, nullptr);

  if (!ReportError(cx, &report, nullptr, nullptr)) {

    return false;

  }



  return report.isWarning();

}