diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 643be63ffa1c..b8b6c5d4b852 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -192,12 +192,30 @@ export function trackUsedThenable( // a listener that will update its status and result when it resolves. switch (thenable.status) { case 'fulfilled': { + // This could be a bad instrumentation that doesn't set .value. + // We're not type-checking since this is a hot path where you can + // track down easily when something becomes `undefined` unexpectedly. const fulfilledValue: T = thenable.value; return fulfilledValue; } case 'rejected': { const rejectedError = thenable.reason; checkIfUseWrappedInAsyncCatch(rejectedError); + + // Rejected Promises are rarer so we're doing an extra type-check in + // case of a bad instrumentation that doesn't set .reason + // If we end up throwing `undefined` it becomes hard to track down + // where that throw originated because no callstack would exist. + // React would still have a Component stack but that could only be used + // as an approximation. + if (rejectedError === undefined && !('reason' in thenable)) { + throw new Error( + 'A rejected Promise was passed to React without a `reason` property. ' + + 'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' + + "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.", + ); + } + throw rejectedError; } default: { diff --git a/packages/react-reconciler/src/__tests__/ReactUse-test.js b/packages/react-reconciler/src/__tests__/ReactUse-test.js index 9714cb4a3e2e..7b4bd5c1c61c 100644 --- a/packages/react-reconciler/src/__tests__/ReactUse-test.js +++ b/packages/react-reconciler/src/__tests__/ReactUse-test.js @@ -81,6 +81,30 @@ describe('ReactUse', () => { }); } + function normalizeCodeLocInfo(str) { + return ( + str && + str.replace( + /^ +(?:at|in) ([\S]+)([^\n]*)(\n?)/gm, + function (m, name, location, eol) { + if (location.indexOf(__filename) === -1) { + // ignore frames from library code + return ''; + } + return ( + ' in ' + + name + + (/:\d+:\d+/.test(m) + ? ' ' + + location.replace(__dirname, '~').replace(/:\d+:\d+/, ':*:*') + : '') + + eol + ); + }, + ) + ); + } + function Text({text}) { Scheduler.log(text); return text; @@ -2189,4 +2213,108 @@ describe('ReactUse', () => { expect(root).toMatchRenderedOutput('Updated'); }, ); + + it('throws a descriptive error when a rejected promise without a reason property is passed to use()', async () => { + const badThenable = Promise.resolve(); + // Simulate bad instrumentation: status is 'rejected' but the reason is not + // in the `reason` property as intended. + badThenable.status = 'rejected'; + badThenable.error = new Error('Something went wrong'); + + function Child() { + return use(badThenable); + } + + function App({ + // Intentionally destrucutring a prop here so that our production error + // stack trick is triggered at the beginning of the function + prop, + }) { + return ; + } + + const uncaughtErrors = []; + const root = ReactNoop.createRoot({ + onUncaughtError(error, errorInfo) { + uncaughtErrors.push({ + callStack: normalizeCodeLocInfo(error.stack), + ownerStack: React.captureOwnerStack + ? normalizeCodeLocInfo(React.captureOwnerStack()) + : null, + componentStack: normalizeCodeLocInfo(errorInfo.componentStack), + }); + }, + }); + + await act(() => { + root.render(); + }); + + expect(uncaughtErrors).toEqual([ + { + callStack: + 'Error: A rejected Promise was passed to React without a `reason` property. ' + + 'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' + + "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`." + + '\n in Child (~/ReactUse-test.js:*:*)' + + '\n in Object. (~/ReactUse-test.js:*:*)', + componentStack: + '\n in Child (~/ReactUse-test.js:*:*)' + + '\n in App (~/ReactUse-test.js:*:*)', + ownerStack: __DEV__ ? '\n in App (~/ReactUse-test.js:*:*)' : null, + }, + ]); + }); + + it('throws a descriptive error when a rejected promise without a reason property is used as a child', async () => { + const badThenable = Promise.resolve(); + // Simulate bad instrumentation: status is 'rejected' but the reason is not + // in the `reason` property as intended. + badThenable.status = 'rejected'; + badThenable.error = new Error('Something went wrong'); + + function Child() { + return
{badThenable}
; + } + + function App({ + // Intentionally destrucutring a prop here so that our production error + // stack trick is triggered at the beginning of the function + prop, + }) { + return ; + } + + const uncaughtErrors = []; + const root = ReactNoop.createRoot({ + onUncaughtError(error, errorInfo) { + uncaughtErrors.push({ + callStack: normalizeCodeLocInfo(error.stack), + ownerStack: React.captureOwnerStack + ? normalizeCodeLocInfo(React.captureOwnerStack()) + : null, + componentStack: normalizeCodeLocInfo(errorInfo.componentStack), + }); + }, + }); + + await act(() => { + root.render(); + }); + + expect(uncaughtErrors).toEqual([ + { + callStack: + 'Error: A rejected Promise was passed to React without a `reason` property. ' + + 'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' + + "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`." + + '\n in Object. (~/ReactUse-test.js:*:*)', + componentStack: '\n in App (~/ReactUse-test.js:*:*)', + ownerStack: __DEV__ + ? // TODO: Should start in Child since that's the owner. + '\n in App (~/ReactUse-test.js:*:*)' + : null, + }, + ]); + }); }); diff --git a/packages/react-server/src/ReactFizzThenable.js b/packages/react-server/src/ReactFizzThenable.js index 6849f16e397d..65eeddc4962f 100644 --- a/packages/react-server/src/ReactFizzThenable.js +++ b/packages/react-server/src/ReactFizzThenable.js @@ -69,11 +69,29 @@ export function trackUsedThenable( // a listener that will update its status and result when it resolves. switch (thenable.status) { case 'fulfilled': { + // This could be a bad instrumentation that doesn't set .value. + // We're not type-checking since this is a hot path where you can + // track down easily when something becomes `undefined` unexpectedly. const fulfilledValue: T = thenable.value; return fulfilledValue; } case 'rejected': { const rejectedError = thenable.reason; + + // Rejected Promises are rarer so we're doing an extra type-check in + // case of a bad instrumentation that doesn't set .reason + // If we end up throwing `undefined` it becomes hard to track down + // where that throw originated because no callstack would exist. + // React would still have a Component stack but that could only be used + // as an approximation. + if (rejectedError === undefined && !('reason' in thenable)) { + throw new Error( + 'A rejected Promise was passed to React without a `reason` property. ' + + 'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' + + "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.", + ); + } + throw rejectedError; } default: { diff --git a/packages/react-server/src/ReactFlightThenable.js b/packages/react-server/src/ReactFlightThenable.js index 99ddb36fa599..9c0c8082facb 100644 --- a/packages/react-server/src/ReactFlightThenable.js +++ b/packages/react-server/src/ReactFlightThenable.js @@ -77,11 +77,29 @@ export function trackUsedThenable( // a listener that will update its status and result when it resolves. switch (thenable.status) { case 'fulfilled': { + // This could be a bad instrumentation that doesn't set .value. + // We're not type-checking since this is a hot path where you can + // track down easily when something becomes `undefined` unexpectedly. const fulfilledValue: T = thenable.value; return fulfilledValue; } case 'rejected': { const rejectedError = thenable.reason; + + // Rejected Promises are rarer so we're doing an extra type-check in + // case of a bad instrumentation that doesn't set .reason + // If we end up throwing `undefined` it becomes hard to track down + // where that throw originated because no callstack would exist. + // React would still have a Component stack but that could only be used + // as an approximation. + if (rejectedError === undefined && !('reason' in thenable)) { + throw new Error( + 'A rejected Promise was passed to React without a `reason` property. ' + + 'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' + + "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.", + ); + } + throw rejectedError; } default: { diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 345aa2b0aaf0..87fd927000c6 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -582,5 +582,6 @@ "594": "Cannot read Symbol exports. Only named exports are supported on a client module imported on the server.", "595": "Attempted to call %s() from the server but %s is on the client. It's not possible to invoke a client function from the server, it can only be rendered as a Component or passed to props of a Client Component.", "596": "Could not find the module \"%s\" in the React Client Manifest. This is probably a bug in the React Server Components bundler.", - "597": "The module \"%s\" is marked as an async ESM module but was loaded as a CJS proxy. This is probably a bug in the React Server Components bundler." -} \ No newline at end of file + "597": "The module \"%s\" is marked as an async ESM module but was loaded as a CJS proxy. This is probably a bug in the React Server Components bundler.", + "598": "A rejected Promise was passed to React without a `reason` property. React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`." +}