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.

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

Re-throw module.error when instantiating runtime modules #4918

Merged
merged 2 commits into from
May 15, 2023

Conversation

alexkirsz
Copy link
Contributor

@alexkirsz alexkirsz commented May 12, 2023

Description

Runtime modules can be executed multiple times within the same context. However, when such a module encounters a runtime error during execution, this error is cached in module.error and not properly re-thrown when the module is required again from getOrInstantiateRuntimeModule, which means that on later runs we’ll silently fail to instantiate the module, and run into any errors that expects a global side-effect from the module.

link WEB-1045

@alexkirsz alexkirsz requested a review from a team as a code owner May 12, 2023 13:11
@vercel
Copy link

vercel bot commented May 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-kitchensink-blog 🔄 Building (Inspect) May 12, 2023 2:39pm
10 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-cra-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-designsystem-docs ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-gatsby-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-native-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-nonmonorepo ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-svelte-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-tailwind-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
examples-vite-web ⬜️ Ignored (Inspect) May 12, 2023 2:39pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview May 12, 2023 2:39pm

@github-actions
Copy link
Contributor

✅ This change can build next-swc

} catch (error) {
module.error = error;
throw error;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is purely aesthetic.

throw e;
} finally {
// Always cleanup the intercept, even if module execution failed.
cleanupReactRefreshIntercept();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change isn't related to the issue, but since we don't expect the runtime to completely crash and burn after a module instantiation error occurs, we should still clean up any side-effects.

@@ -1418,6 +1424,9 @@ function instantiateRuntimeModule(moduleId, chunkPath) {
function getOrInstantiateRuntimeModule(moduleId, chunkPath) {
const module = moduleCache[moduleId];
if (module) {
if (module.error) {
throw module.error;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix.

@github-actions
Copy link
Contributor

github-actions bot commented May 12, 2023

🟢 CI successful 🟢

Thanks

@alexkirsz alexkirsz added the pr: automerge Kodiak will merge these automatically after checks pass label May 15, 2023
@kodiakhq kodiakhq bot merged commit 7d2a57f into main May 15, 2023
@kodiakhq kodiakhq bot deleted the alexkirsz/web-1045-dashboard-cannot-read-properties-of branch May 15, 2023 08:27
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…orepo#4918)

### Description

Runtime modules can be executed multiple times within the same context.
However, when such a module encounters a runtime error during execution,
this error is cached in module.error and not properly re-thrown when the
module is required again from `getOrInstantiateRuntimeModule`, which
means that on later runs we’ll silently fail to instantiate the module,
and run into any errors that expects a global side-effect from the
module.

link WEB-1045
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…orepo#4918)

### Description

Runtime modules can be executed multiple times within the same context.
However, when such a module encounters a runtime error during execution,
this error is cached in module.error and not properly re-thrown when the
module is required again from `getOrInstantiateRuntimeModule`, which
means that on later runs we’ll silently fail to instantiate the module,
and run into any errors that expects a global side-effect from the
module.

link WEB-1045
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…orepo#4918)

### Description

Runtime modules can be executed multiple times within the same context.
However, when such a module encounters a runtime error during execution,
this error is cached in module.error and not properly re-thrown when the
module is required again from `getOrInstantiateRuntimeModule`, which
means that on later runs we’ll silently fail to instantiate the module,
and run into any errors that expects a global side-effect from the
module.

link WEB-1045
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: automerge Kodiak will merge these automatically after checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants