Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Construct Error at invariant call site for clearer stack traces #15877

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Construct Error at invariant call site for clearer stack traces #15877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions packages/shared/ReactError.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
*
*/

// Do not require this module directly! Use a normal error constructor with
// Do not require this module directly! Use normal `invariant` calls with
// template literal strings. The messages will be converted to ReactError during
// build, and in production they will be minified.

function ReactError(message) {
const error = new Error(message);
function ReactError(error) {
error.name = 'Invariant Violation';
return error;
}
Expand Down
13 changes: 7 additions & 6 deletions packages/shared/ReactErrorProd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,21 @@
*
*/

// Do not require this module directly! Use a normal error constructor with
// Do not require this module directly! Use normal `invariant` calls with
// template literal strings. The messages will be converted to ReactError during
// build, and in production they will be minified.

function ReactErrorProd(code) {
function ReactErrorProd(error) {
const code = error.message;
let url = 'https://reactjs.org/docs/error-decoder.html?invariant=' + code;
for (let i = 1; i < arguments.length; i++) {
url += '&args[]=' + encodeURIComponent(arguments[i]);
}
return new Error(
error.message =
`Minified React error #${code}; visit ${url} for the full message or ` +
'use the non-minified dev environment for full errors and additional ' +
'helpful warnings. ',
);
'use the non-minified dev environment for full errors and additional ' +
'helpful warnings. ';
return error;
}

export default ReactErrorProd;
6 changes: 3 additions & 3 deletions packages/shared/__tests__/ReactErrorProd-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('ReactErrorProd', () => {

it('should throw with the correct number of `%s`s in the URL', () => {
expect(function() {
throw ReactErrorProd(124, 'foo', 'bar');
throw ReactErrorProd(Error(124), 'foo', 'bar');
}).toThrowError(
'Minified React error #124; visit ' +
'https://reactjs.org/docs/error-decoder.html?invariant=124&args[]=foo&args[]=bar' +
Expand All @@ -45,7 +45,7 @@ describe('ReactErrorProd', () => {
);

expect(function() {
throw ReactErrorProd(20);
throw ReactErrorProd(Error(20));
}).toThrowError(
'Minified React error #20; visit ' +
'https://reactjs.org/docs/error-decoder.html?invariant=20' +
Expand All @@ -54,7 +54,7 @@ describe('ReactErrorProd', () => {
);

expect(function() {
throw ReactErrorProd(77, '<div>', '&?bar');
throw ReactErrorProd(Error(77), '<div>', '&?bar');
}).toThrowError(
'Minified React error #77; visit ' +
'https://reactjs.org/docs/error-decoder.html?invariant=77&args[]=%3Cdiv%3E&args[]=%26%3Fbar' +
Expand Down
26 changes: 13 additions & 13 deletions scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ exports[`error transform should correctly transform invariants that are not in t
import invariant from 'shared/invariant';
/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/(function () {
if (!condition) {
throw _ReactError(\`This is not a real error message.\`);
throw _ReactError(Error(\`This is not a real error message.\`));
}
})();"
`;
Expand All @@ -17,7 +17,7 @@ exports[`error transform should handle escaped characters 1`] = `
import invariant from 'shared/invariant';
/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/(function () {
if (!condition) {
throw _ReactError(\`What's up?\`);
throw _ReactError(Error(\`What's up?\`));
}
})();"
`;
Expand All @@ -30,18 +30,18 @@ import invariant from 'shared/invariant';
(function () {
if (!condition) {
if (__DEV__) {
throw _ReactError(\`Do not override existing functions.\`);
throw _ReactError(Error(\`Do not override existing functions.\`));
} else {
throw _ReactErrorProd(16);
throw _ReactErrorProd(Error(16));
}
}
})();
(function () {
if (!condition) {
if (__DEV__) {
throw _ReactError(\`Do not override existing functions.\`);
throw _ReactError(Error(\`Do not override existing functions.\`));
} else {
throw _ReactErrorProd(16);
throw _ReactErrorProd(Error(16));
}
}
})();"
Expand All @@ -55,9 +55,9 @@ import invariant from 'shared/invariant';
(function () {
if (!condition) {
if (__DEV__) {
throw _ReactError(\`Do not override existing functions.\`);
throw _ReactError(Error(\`Do not override existing functions.\`));
} else {
throw _ReactErrorProd(16);
throw _ReactErrorProd(Error(16));
}
}
})();"
Expand All @@ -71,9 +71,9 @@ import invariant from 'shared/invariant';
(function () {
if (!condition) {
if (__DEV__) {
throw _ReactError(\`Expected a component class, got \${Foo}.\${Bar}\`);
throw _ReactError(Error(\`Expected a component class, got \${Foo}.\${Bar}\`));
} else {
throw _ReactErrorProd(18, Foo, Bar);
throw _ReactErrorProd(Error(18), Foo, Bar);
}
}
})();"
Expand All @@ -87,9 +87,9 @@ import invariant from 'shared/invariant';
(function () {
if (!condition) {
if (__DEV__) {
throw _ReactError(\`Expected \${foo} target to be an array; got \${bar}\`);
throw _ReactError(Error(\`Expected \${foo} target to be an array; got \${bar}\`));
} else {
throw _ReactErrorProd(7, foo, bar);
throw _ReactErrorProd(Error(7), foo, bar);
}
}
})();"
Expand All @@ -101,7 +101,7 @@ exports[`error transform should support noMinify option 1`] = `
import invariant from 'shared/invariant';
(function () {
if (!condition) {
throw _ReactError(\`Do not override existing functions.\`);
throw _ReactError(Error(\`Do not override existing functions.\`));
}
})();"
`;
24 changes: 14 additions & 10 deletions scripts/error-codes/transform-error-messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ module.exports = function(babel) {
//
// if (!condition) {
// if (__DEV__) {
// throw ReactError(`A ${adj} message that contains ${noun}`);
// throw ReactError(Error(`A ${adj} message that contains ${noun}`));
// } else {
// throw ReactErrorProd(ERR_CODE, adj, noun);
// throw ReactErrorProd(Error(ERR_CODE), adj, noun);
// }
// }
//
Expand All @@ -53,10 +53,12 @@ module.exports = function(babel) {
);

// Outputs:
// throw ReactError(`A ${adj} message that contains ${noun}`);
// throw ReactError(Error(`A ${adj} message that contains ${noun}`));
const devThrow = t.throwStatement(
t.callExpression(reactErrorIdentfier, [
t.templateLiteral(errorMsgQuasis, errorMsgExpressions),
t.callExpression(t.identifier('Error'), [
t.templateLiteral(errorMsgQuasis, errorMsgExpressions),
]),
])
);

Expand All @@ -65,7 +67,7 @@ module.exports = function(babel) {
//
// Outputs:
// if (!condition) {
// throw ReactError(`A ${adj} message that contains ${noun}`);
// throw ReactError(Error(`A ${adj} message that contains ${noun}`));
// }
path.replaceWith(
t.ifStatement(
Expand All @@ -92,7 +94,7 @@ module.exports = function(babel) {
// Outputs:
// /* FIXME (minify-errors-in-prod): Unminified error message in production build! */
// if (!condition) {
// throw ReactError(`A ${adj} message that contains ${noun}`);
// throw ReactError(Error(`A ${adj} message that contains ${noun}`));
// }
path.replaceWith(
t.ifStatement(
Expand All @@ -116,20 +118,22 @@ module.exports = function(babel) {
);

// Outputs:
// throw ReactErrorProd(ERR_CODE, adj, noun);
// throw ReactErrorProd(Error(ERR_CODE), adj, noun);
const prodThrow = t.throwStatement(
t.callExpression(reactErrorProdIdentfier, [
t.numericLiteral(prodErrorId),
t.callExpression(t.identifier('Error'), [
t.numericLiteral(prodErrorId),
]),
...errorMsgExpressions,
])
);

// Outputs:
// if (!condition) {
// if (__DEV__) {
// throw ReactError(`A ${adj} message that contains ${noun}`);
// throw ReactError(Error(`A ${adj} message that contains ${noun}`));
// } else {
// throw ReactErrorProd(ERR_CODE, adj, noun);
// throw ReactErrorProd(Error(ERR_CODE), adj, noun);
// }
// }
path.replaceWith(
Expand Down
43 changes: 41 additions & 2 deletions scripts/jest/setupTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
if (process.env.NODE_ENV === 'production') {
// In production, we strip error messages and turn them into codes.
// This decodes them back so that the test assertions on them work.
// 1. `ErrorProxy` decodes error messages at Error construction time and
// also proxies error instances with `proxyErrorInstance`.
// 2. `proxyErrorInstance` decodes error messages when the `message`
// property is changed.
const decodeErrorMessage = function(message) {
if (!message) {
return message;
Expand All @@ -150,16 +154,51 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) {
return format.replace(/%s/g, () => args[argIndex++]);
};
const OriginalError = global.Error;
// V8's Error.captureStackTrace (used in Jest) fails if the error object is
// a Proxy, so we need to pass it the unproxied instance.
const originalErrorInstances = new WeakMap();
const captureStackTrace = function(error, ...args) {
return OriginalError.captureStackTrace.call(
this,
originalErrorInstances.get(error) ||
// Sometimes this wrapper receives an already-unproxied instance.
error,
...args
);
};
const proxyErrorInstance = error => {
const proxy = new Proxy(error, {
set(target, key, value, receiver) {
if (key === 'message') {
return Reflect.set(
target,
key,
decodeErrorMessage(value),
receiver
);
}
return Reflect.set(target, key, value, receiver);
},
});
originalErrorInstances.set(proxy, error);
return proxy;
};
const ErrorProxy = new Proxy(OriginalError, {
apply(target, thisArg, argumentsList) {
const error = Reflect.apply(target, thisArg, argumentsList);
error.message = decodeErrorMessage(error.message);
return error;
return proxyErrorInstance(error);
},
construct(target, argumentsList, newTarget) {
const error = Reflect.construct(target, argumentsList, newTarget);
error.message = decodeErrorMessage(error.message);
return error;
return proxyErrorInstance(error);
},
get(target, key, receiver) {
if (key === 'captureStackTrace') {
return captureStackTrace;
}
return Reflect.get(target, key, receiver);
},
});
ErrorProxy.OriginalError = OriginalError;
Expand Down