Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Add basic support for throws in React #2502

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/completions.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,6 @@ export class ThrowCompletion extends AbruptCompletion {
constructor(value: Value, location: ?BabelNodeSourceLocation, nativeStack?: ?string) {
super(value, location);
this.nativeStack = nativeStack || new Error().stack;
let realm = value.$Realm;
if (realm.isInPureScope()) {
for (let callback of realm.reportSideEffectCallbacks) {
callback("EXCEPTION_THROWN", undefined, location);
}
}
}

nativeStack: string;
Expand Down
34 changes: 7 additions & 27 deletions src/react/reconcilation.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import {
getValueWithBranchingLogicApplied,
wrapReactElementInBranchOrReturnValue,
} from "./branching.js";
import { Completion, SimpleNormalCompletion } from "../completions.js";
import { AbruptCompletion, SimpleNormalCompletion } from "../completions.js";
import {
getInitialProps,
getInitialContext,
Expand Down Expand Up @@ -179,6 +179,7 @@ export class Reconciler {
this.statistics.optimizedTrees++;
return result;
} catch (error) {
if (error instanceof AbruptCompletion) throw error;
this._handleComponentTreeRootFailure(error, evaluatedRootNode);
// flow belives we can get here, when it should never be possible
invariant(false, "resolveReactComponentTree error not handled correctly");
Expand Down Expand Up @@ -1224,6 +1225,7 @@ export class Reconciler {

return result;
} catch (error) {
if (error instanceof AbruptCompletion) throw error;
return this._resolveComponentResolutionFailure(
componentType,
error,
Expand All @@ -1235,7 +1237,7 @@ export class Reconciler {
}
}

_handleComponentTreeRootFailure(error: Error | Completion, evaluatedRootNode: ReactEvaluatedNode): void {
_handleComponentTreeRootFailure(error: Error, evaluatedRootNode: ReactEvaluatedNode): void {
if (error.name === "Invariant Violation") {
throw error;
} else if (error instanceof ReconcilerFatalError) {
Expand All @@ -1245,19 +1247,6 @@ export class Reconciler {
`Failed to render React component root "${evaluatedRootNode.name}" due to ${error.message}`,
evaluatedRootNode
);
} else if (error instanceof Completion) {
let value = error.value;
invariant(value instanceof ObjectValue);
let message = getProperty(this.realm, value, "message");
let stack = getProperty(this.realm, value, "stack");
invariant(message instanceof StringValue);
invariant(stack instanceof StringValue);
throw new ReconcilerFatalError(
`Failed to render React component "${evaluatedRootNode.name}" due to a JS error: ${message.value}\n${
stack.value
}`,
evaluatedRootNode
);
}
let message;
if (error instanceof ExpectedBailOut) {
Expand All @@ -1277,7 +1266,7 @@ export class Reconciler {

_resolveComponentResolutionFailure(
componentType: Value,
error: Error | Completion,
error: Error,
reactElement: ObjectValue,
context: ObjectValue | AbstractObjectValue,
evaluatedNode: ReactEvaluatedNode,
Expand All @@ -1294,17 +1283,6 @@ export class Reconciler {
);
} else if (error instanceof DoNotOptimize) {
return reactElement;
} else if (error instanceof Completion) {
let value = error.value;
invariant(value instanceof ObjectValue);
let message = getProperty(this.realm, value, "message");
let stack = getProperty(this.realm, value, "stack");
invariant(message instanceof StringValue);
invariant(stack instanceof StringValue);
throw new ReconcilerFatalError(
`Failed to render React component "${evaluatedNode.name}" due to a JS error: ${message.value}\n${stack.value}`,
evaluatedNode
);
}
let typeValue = getProperty(this.realm, reactElement, "type");
let propsValue = getProperty(this.realm, reactElement, "props");
Expand Down Expand Up @@ -1418,6 +1396,8 @@ export class Reconciler {
this.realm.applyEffects(effects);
if (result instanceof SimpleNormalCompletion) {
result = result.value;
} else {
invariant(false, "TODO support other types of completion");
}
invariant(result instanceof Value);
return this._resolveDeeply(componentType, result, context, branchStatus, evaluatedNode, needsKey);
Expand Down
3 changes: 0 additions & 3 deletions src/react/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,6 @@ export function getValueFromFunctionCall(
let funcCall = func.$Call;
let newCall = func.$Construct;
let completion;
let createdObjects = realm.createdObjects;
try {
let value;
if (isConstructor) {
Expand All @@ -814,8 +813,6 @@ export function getValueFromFunctionCall(
} else {
throw error;
}
} finally {
invariant(createdObjects === realm.createdObjects, "realm.createdObjects was not correctly restored");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this invariant, it's been very useful in finding early evaluation errors. Rather, we should capture the effects of the function call (using evaluateForEffects) and apply them if the createdObjects has been correctly restored. We ran into countless issues in the past that this guarded against, so I don't think we should regress on this if possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, to make this PR easier I'm happy to remove the invariant for now. I can revisit with some changes later this week that I had planned.

}
return realm.returnOrThrowCompletion(completion);
}
Expand Down
2 changes: 1 addition & 1 deletion test/error-handler/ModifiedObjectPropertyLimitation.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// recover-from-errors
// expected errors: [{"severity":"Warning","errorCode":"PP1007","callStack":"Error\n "},{"severity":"Warning","errorCode":"PP0023","callStack":"Error\n "}]
// expected errors: [{"severity":"Warning","errorCode":"PP0023","callStack":"Error\n "}]
(function() {
let p = {};
function f(c) {
Expand Down
2 changes: 1 addition & 1 deletion test/error-handler/bad-functions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// recover-from-errors
// expected errors: [{"severity":"Warning","errorCode":"PP1007","callStack":"Error\n "},{"severity":"Warning","errorCode":"PP1007","callStack":"Error\n "},{"severity":"Warning","errorCode":"PP0023","callStack":"Error\n "},{"severity":"Warning","errorCode":"PP1007","callStack":"Error\n "},{"location":{"start":{"line":12,"column":13},"end":{"line":12,"column":18},"source":"test/error-handler/bad-functions.js"},"severity":"RecoverableError","errorCode":"PP1003"},{"location":{"start":{"line":8,"column":13},"end":{"line":8,"column":18},"source":"test/error-handler/bad-functions.js"},"severity":"RecoverableError","errorCode":"PP1003"}]
// expected errors: [{"severity":"Warning","errorCode":"PP1007","callStack":"Error\n "},{"severity":"Warning","errorCode":"PP0023","callStack":"Error\n "},{"severity":"Warning","errorCode":"PP1007","callStack":"Error\n "},{"location":{"start":{"line":12,"column":13},"end":{"line":12,"column":18},"source":"test/error-handler/bad-functions.js"},"severity":"RecoverableError","errorCode":"PP1003"},{"location":{"start":{"line":8,"column":13},"end":{"line":8,"column":18},"source":"test/error-handler/bad-functions.js"},"severity":"RecoverableError","errorCode":"PP1003"}]
var wildcard = global.__abstract ? global.__abstract("number", "123") : 123;
global.a = "";

Expand Down
1 change: 1 addition & 0 deletions test/react/FBMocks/fb16.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ __evaluatePureFunction(function() {
function ViewCount(props) {
return React.createElement(
"div",
null,
fbt._({ "*": "{count} Views", _1: "{count} View" }, [
fbt._param("count", props.feedback.viewCountReduced),
fbt._plural(props.feedback.viewCount),
Expand Down
2 changes: 1 addition & 1 deletion test/react/FunctionalComponents-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ it("Simple 12", () => {

it("Runtime error", () => {
runTest(__dirname + "/FunctionalComponents/runtime-error.js", {
expectReconcilerError: true,
expectRuntimeError: true,
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/react/FunctionalComponents/simple-13.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function Child(props) {
return children();
}

__optimizeReactComponentTree(App);
if (this.__optimizeReactComponentTree) __optimizeReactComponentTree(App);

App.getTrials = function(renderer, Root) {
// Just compile, don't run
Expand Down
23 changes: 23 additions & 0 deletions test/react/Throw-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Copyright (c) 2017-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

/* @flow */

const path = require("path");
const fs = require("fs");
const setupReactTests = require("./setupReactTests");
const { runTest } = setupReactTests();

const customConfig = new Map();

fs.readdirSync(path.resolve(__dirname, "Throw")).forEach(file => {
test(file, () => {
runTest(path.resolve(__dirname, "Throw", file), customConfig.get(file));
});
});
15 changes: 15 additions & 0 deletions test/react/Throw/throw-conditional.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const React = require("react");

function MyComponent(props) {
if (props.b) throw new Error("abrupt");
return 42;
}

if (global.__optimizeReactComponentTree) global.__optimizeReactComponentTree(MyComponent);

MyComponent.getTrials = renderer => {
renderer.update(<MyComponent b={false} />);
return [["simple render", renderer.toJSON()]];
};

module.exports = MyComponent;
19 changes: 19 additions & 0 deletions test/react/Throw/throw.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const React = require("react");

function MyComponent() {
throw new Error("abrupt");
}

if (global.__optimizeReactComponentTree) global.__optimizeReactComponentTree(MyComponent);

MyComponent.getTrials = renderer => {
let error = false;
try {
MyComponent({});
} catch (error) {
error = true;
}
return [["component errors", error]];
};

module.exports = MyComponent;
Loading