Re-throw module.error when instantiating runtime modules (vercel/turb…

…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
vercel · May 15, 2023 · 84c42b7 · 84c42b7
1 parent 7b12255
commit 84c42b7
Show file tree

Hide file tree

Showing 36 changed files with 1,044 additions and 720 deletions.
diff --git a/crates/turbopack-dev/js/src/runtime.js b/crates/turbopack-dev/js/src/runtime.js
@@ -482,8 +482,9 @@ function instantiateModule(id, source) {
       break;
   }
 
-  runModuleExecutionHooks(module, (refresh) => {
-    try {
+  // NOTE(alexkirsz) This can fail when the module encounters a runtime error.
+  try {
+    runModuleExecutionHooks(module, (refresh) => {
       moduleFactory.call(module.exports, {
         e: module.exports,
         r: commonJsRequire.bind(null, module),
@@ -501,11 +502,11 @@ function instantiateModule(id, source) {
         k: refresh,
         __dirname: module.id.replace(/(^|\/)[\/]+$/, ""),
       });
-    } catch (error) {
-      module.error = error;
-      throw error;
-    }
-  });
+    });
+  } catch (error) {
+    module.error = error;
+    throw error;
+  }
 
   module.loaded = true;
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
@@ -530,21 +531,26 @@ function runModuleExecutionHooks(module, executeModule) {
       ? globalThis.$RefreshInterceptModuleExecution$(module.id)
       : () => {};
 
-  executeModule({
-    register: globalThis.$RefreshReg$,
-    signature: globalThis.$RefreshSig$,
-  });
+  try {
+    executeModule({
+      register: globalThis.$RefreshReg$,
+      signature: globalThis.$RefreshSig$,
+    });
 
-  if ("$RefreshHelpers$" in globalThis) {
-    // This pattern can also be used to register the exports of
-    // a module with the React Refresh runtime.
-    registerExportsAndSetupBoundaryForReactRefresh(
-      module,
-      globalThis.$RefreshHelpers$
-    );
+    if ("$RefreshHelpers$" in globalThis) {
+      // This pattern can also be used to register the exports of
+      // a module with the React Refresh runtime.
+      registerExportsAndSetupBoundaryForReactRefresh(
+        module,
+        globalThis.$RefreshHelpers$
+      );
+    }
+  } catch (e) {
+    throw e;
+  } finally {
+    // Always cleanup the intercept, even if module execution failed.
+    cleanupReactRefreshIntercept();
   }
-
-  cleanupReactRefreshIntercept();
 }
 
 /**
@@ -1418,6 +1424,9 @@ function instantiateRuntimeModule(moduleId, chunkPath) {
 function getOrInstantiateRuntimeModule(moduleId, chunkPath) {
   const module = moduleCache[moduleId];
   if (module) {
+    if (module.error) {
+      throw module.error;
+    }
     return module;
   }
 

diff --git a/...hunk/output/crates_turbopack-tests_tests_snapshot_basic_async_chunk_input_index_0d348e.js b/...hunk/output/crates_turbopack-tests_tests_snapshot_basic_async_chunk_input_index_0d348e.js
@@ -491,8 +491,9 @@ function instantiateModule(id, source) {
       break;
   }
 
-  runModuleExecutionHooks(module, (refresh) => {
-    try {
+  // NOTE(alexkirsz) This can fail when the module encounters a runtime error.
+  try {
+    runModuleExecutionHooks(module, (refresh) => {
       moduleFactory.call(module.exports, {
         e: module.exports,
         r: commonJsRequire.bind(null, module),
@@ -510,11 +511,11 @@ function instantiateModule(id, source) {
         k: refresh,
         __dirname: module.id.replace(/(^|\/)[\/]+$/, ""),
       });
-    } catch (error) {
-      module.error = error;
-      throw error;
-    }
-  });
+    });
+  } catch (error) {
+    module.error = error;
+    throw error;
+  }
 
   module.loaded = true;
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
@@ -539,21 +540,26 @@ function runModuleExecutionHooks(module, executeModule) {
       ? globalThis.$RefreshInterceptModuleExecution$(module.id)
       : () => {};
 
-  executeModule({
-    register: globalThis.$RefreshReg$,
-    signature: globalThis.$RefreshSig$,
-  });
+  try {
+    executeModule({
+      register: globalThis.$RefreshReg$,
+      signature: globalThis.$RefreshSig$,
+    });
 
-  if ("$RefreshHelpers$" in globalThis) {
-    // This pattern can also be used to register the exports of
-    // a module with the React Refresh runtime.
-    registerExportsAndSetupBoundaryForReactRefresh(
-      module,
-      globalThis.$RefreshHelpers$
-    );
+    if ("$RefreshHelpers$" in globalThis) {
+      // This pattern can also be used to register the exports of
+      // a module with the React Refresh runtime.
+      registerExportsAndSetupBoundaryForReactRefresh(
+        module,
+        globalThis.$RefreshHelpers$
+      );
+    }
+  } catch (e) {
+    throw e;
+  } finally {
+    // Always cleanup the intercept, even if module execution failed.
+    cleanupReactRefreshIntercept();
   }
-
-  cleanupReactRefreshIntercept();
 }
 
 /**
@@ -1427,6 +1433,9 @@ function instantiateRuntimeModule(moduleId, chunkPath) {
 function getOrInstantiateRuntimeModule(moduleId, chunkPath) {
   const module = moduleCache[moduleId];
   if (module) {
+    if (module.error) {
+      throw module.error;
+    }
     return module;
   }
 

diff --git a/.../chunked/output/crates_turbopack-tests_tests_snapshot_basic_chunked_input_index_e77e9f.js b/.../chunked/output/crates_turbopack-tests_tests_snapshot_basic_chunked_input_index_e77e9f.js
@@ -491,8 +491,9 @@ function instantiateModule(id, source) {
       break;
   }
 
-  runModuleExecutionHooks(module, (refresh) => {
-    try {
+  // NOTE(alexkirsz) This can fail when the module encounters a runtime error.
+  try {
+    runModuleExecutionHooks(module, (refresh) => {
       moduleFactory.call(module.exports, {
         e: module.exports,
         r: commonJsRequire.bind(null, module),
@@ -510,11 +511,11 @@ function instantiateModule(id, source) {
         k: refresh,
         __dirname: module.id.replace(/(^|\/)[\/]+$/, ""),
       });
-    } catch (error) {
-      module.error = error;
-      throw error;
-    }
-  });
+    });
+  } catch (error) {
+    module.error = error;
+    throw error;
+  }
 
   module.loaded = true;
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
@@ -539,21 +540,26 @@ function runModuleExecutionHooks(module, executeModule) {
       ? globalThis.$RefreshInterceptModuleExecution$(module.id)
       : () => {};
 
-  executeModule({
-    register: globalThis.$RefreshReg$,
-    signature: globalThis.$RefreshSig$,
-  });
+  try {
+    executeModule({
+      register: globalThis.$RefreshReg$,
+      signature: globalThis.$RefreshSig$,
+    });
 
-  if ("$RefreshHelpers$" in globalThis) {
-    // This pattern can also be used to register the exports of
-    // a module with the React Refresh runtime.
-    registerExportsAndSetupBoundaryForReactRefresh(
-      module,
-      globalThis.$RefreshHelpers$
-    );
+    if ("$RefreshHelpers$" in globalThis) {
+      // This pattern can also be used to register the exports of
+      // a module with the React Refresh runtime.
+      registerExportsAndSetupBoundaryForReactRefresh(
+        module,
+        globalThis.$RefreshHelpers$
+      );
+    }
+  } catch (e) {
+    throw e;
+  } finally {
+    // Always cleanup the intercept, even if module execution failed.
+    cleanupReactRefreshIntercept();
   }
-
-  cleanupReactRefreshIntercept();
 }
 
 /**
@@ -1427,6 +1433,9 @@ function instantiateRuntimeModule(moduleId, chunkPath) {
 function getOrInstantiateRuntimeModule(moduleId, chunkPath) {
   const module = moduleCache[moduleId];
   if (module) {
+    if (module.error) {
+      throw module.error;
+    }
     return module;
   }
 

diff --git a/.../shebang/output/crates_turbopack-tests_tests_snapshot_basic_shebang_input_index_b1f0c2.js b/.../shebang/output/crates_turbopack-tests_tests_snapshot_basic_shebang_input_index_b1f0c2.js
@@ -491,8 +491,9 @@ function instantiateModule(id, source) {
       break;
   }
 
-  runModuleExecutionHooks(module, (refresh) => {
-    try {
+  // NOTE(alexkirsz) This can fail when the module encounters a runtime error.
+  try {
+    runModuleExecutionHooks(module, (refresh) => {
       moduleFactory.call(module.exports, {
         e: module.exports,
         r: commonJsRequire.bind(null, module),
@@ -510,11 +511,11 @@ function instantiateModule(id, source) {
         k: refresh,
         __dirname: module.id.replace(/(^|\/)[\/]+$/, ""),
       });
-    } catch (error) {
-      module.error = error;
-      throw error;
-    }
-  });
+    });
+  } catch (error) {
+    module.error = error;
+    throw error;
+  }
 
   module.loaded = true;
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
@@ -539,21 +540,26 @@ function runModuleExecutionHooks(module, executeModule) {
       ? globalThis.$RefreshInterceptModuleExecution$(module.id)
       : () => {};
 
-  executeModule({
-    register: globalThis.$RefreshReg$,
-    signature: globalThis.$RefreshSig$,
-  });
+  try {
+    executeModule({
+      register: globalThis.$RefreshReg$,
+      signature: globalThis.$RefreshSig$,
+    });
 
-  if ("$RefreshHelpers$" in globalThis) {
-    // This pattern can also be used to register the exports of
-    // a module with the React Refresh runtime.
-    registerExportsAndSetupBoundaryForReactRefresh(
-      module,
-      globalThis.$RefreshHelpers$
-    );
+    if ("$RefreshHelpers$" in globalThis) {
+      // This pattern can also be used to register the exports of
+      // a module with the React Refresh runtime.
+      registerExportsAndSetupBoundaryForReactRefresh(
+        module,
+        globalThis.$RefreshHelpers$
+      );
+    }
+  } catch (e) {
+    throw e;
+  } finally {
+    // Always cleanup the intercept, even if module execution failed.
+    cleanupReactRefreshIntercept();
   }
-
-  cleanupReactRefreshIntercept();
 }
 
 /**
@@ -1427,6 +1433,9 @@ function instantiateRuntimeModule(moduleId, chunkPath) {
 function getOrInstantiateRuntimeModule(moduleId, chunkPath) {
   const module = moduleCache[moduleId];
   if (module) {
+    if (module.error) {
+      throw module.error;
+    }
     return module;
   }
 

diff --git a/...define/output/crates_turbopack-tests_tests_snapshot_comptime_define_input_index_6b0d2b.js b/...define/output/crates_turbopack-tests_tests_snapshot_comptime_define_input_index_6b0d2b.js
@@ -491,8 +491,9 @@ function instantiateModule(id, source) {
       break;
   }
 
-  runModuleExecutionHooks(module, (refresh) => {
-    try {
+  // NOTE(alexkirsz) This can fail when the module encounters a runtime error.
+  try {
+    runModuleExecutionHooks(module, (refresh) => {
       moduleFactory.call(module.exports, {
         e: module.exports,
         r: commonJsRequire.bind(null, module),
@@ -510,11 +511,11 @@ function instantiateModule(id, source) {
         k: refresh,
         __dirname: module.id.replace(/(^|\/)[\/]+$/, ""),
       });
-    } catch (error) {
-      module.error = error;
-      throw error;
-    }
-  });
+    });
+  } catch (error) {
+    module.error = error;
+    throw error;
+  }
 
   module.loaded = true;
   if (module.namespaceObject && module.exports !== module.namespaceObject) {
@@ -539,21 +540,26 @@ function runModuleExecutionHooks(module, executeModule) {
       ? globalThis.$RefreshInterceptModuleExecution$(module.id)
       : () => {};
 
-  executeModule({
-    register: globalThis.$RefreshReg$,
-    signature: globalThis.$RefreshSig$,
-  });
+  try {
+    executeModule({
+      register: globalThis.$RefreshReg$,
+      signature: globalThis.$RefreshSig$,
+    });
 
-  if ("$RefreshHelpers$" in globalThis) {
-    // This pattern can also be used to register the exports of
-    // a module with the React Refresh runtime.
-    registerExportsAndSetupBoundaryForReactRefresh(
-      module,
-      globalThis.$RefreshHelpers$
-    );
+    if ("$RefreshHelpers$" in globalThis) {
+      // This pattern can also be used to register the exports of
+      // a module with the React Refresh runtime.
+      registerExportsAndSetupBoundaryForReactRefresh(
+        module,
+        globalThis.$RefreshHelpers$
+      );
+    }
+  } catch (e) {
+    throw e;
+  } finally {
+    // Always cleanup the intercept, even if module execution failed.
+    cleanupReactRefreshIntercept();
   }
-
-  cleanupReactRefreshIntercept();
 }
 
 /**
@@ -1427,6 +1433,9 @@ function instantiateRuntimeModule(moduleId, chunkPath) {
 function getOrInstantiateRuntimeModule(moduleId, chunkPath) {
   const module = moduleCache[moduleId];
   if (module) {
+    if (module.error) {
+      throw module.error;
+    }
     return module;
   }