fix: importModule should receive error correctly (#8827)

crates/node_binding/binding.d.ts

@@ -653,6 +653,7 @@ export interface JsExecuteModuleResult {
   cacheable: boolean
   assets: Array<string>
   id: number
+  error?: string
 }
 
 export interface JsFactorizeArgs {

crates/node_binding/package.json

@@ -51,4 +51,4 @@
     "@rspack/binding-linux-x64-gnu": "workspace:*",
     "@rspack/binding-win32-x64-msvc": "workspace:*"
   }
-}
+}

crates/rspack_binding_values/src/compilation/mod.rs

@@ -683,6 +683,7 @@ impl JsCompilation {
           .collect(),
         assets: res.assets.into_iter().collect(),
         id: res.id,
+        error: res.error,
       };
       Ok(js_result)
     })
@@ -890,6 +891,7 @@ pub struct JsExecuteModuleResult {
   pub cacheable: bool,
   pub assets: Vec<String>,
   pub id: u32,
+  pub error: Option<String>,
 }
 
 #[napi(object)]

packages/rspack-test-tools/tests/configCases/loader-import-module/execute-failed/import-loader-2.js

@@ -0,0 +1,7 @@
+module.exports.pitch = function () {
+	const callback = this.async();
+	this.importModule(`${this.resourcePath}.webpack[javascript/auto]!=!!!./index.js`, {}, err=> {
+		expect(err.message).toBe('Error: execute failed')
+		callback(null, `export default "${err}"`);
+	})
+}

packages/rspack-test-tools/tests/configCases/loader-import-module/execute-failed/import-loader.js

@@ -0,0 +1,10 @@
+module.exports.pitch = function () {
+	const callback = this.async();
+	this.importModule(`${this.resourcePath}.webpack[javascript/auto]!=!!!./index.js`, {}).then((_exports) => {
+		throw new Error("This should not be executed");
+	}).catch((err) => {
+		expect(err.message).toBe('Error: execute failed')
+		// expect(err).toBe('execute failed')
+		callback(null, `export default "${err}"`);
+	})
+}

packages/rspack-test-tools/tests/configCases/loader-import-module/execute-failed/index.js

@@ -0,0 +1 @@
+throw new Error('execute failed')

packages/rspack-test-tools/tests/configCases/loader-import-module/execute-failed/rspack.config.js

@@ -0,0 +1,12 @@
+/** @type {import("@rspack/core").Configuration} */
+module.exports = {
+	entry: "./index.js",
+	module: {
+		rules: [
+			{
+				test: /index\.js/,
+				use: ['./import-loader.js', './import-loader-2.js'],
+			},
+		]
+	},
+};

packages/rspack/src/Compiler.ts

@@ -1061,69 +1061,73 @@ class Compiler {
 						codegenResults,
 						runtimeModules
 					}: binding.JsExecuteModuleArg) {
-						const __webpack_require__: any = (id: string) => {
-							const cached = moduleCache[id];
-							if (cached !== undefined) {
-								if (cached.error) throw cached.error;
-								return cached.exports;
-							}
+						try {
+							const __webpack_require__: any = (id: string) => {
+								const cached = moduleCache[id];
+								if (cached !== undefined) {
+									if (cached.error) throw cached.error;
+									return cached.exports;
+								}
 
-							const execOptions = {
-								id,
-								module: {
+								const execOptions = {
 									id,
-									exports: {},
-									loaded: false,
-									error: undefined
-								},
-								require: __webpack_require__
-							};
-
-							for (const handler of interceptModuleExecution) {
-								handler(execOptions);
-							}
+									module: {
+										id,
+										exports: {},
+										loaded: false,
+										error: undefined
+									},
+									require: __webpack_require__
+								};
+
+								for (const handler of interceptModuleExecution) {
+									handler(execOptions);
+								}
 
-							const result = codegenResults.map[id]["build time"];
-							const moduleObject = execOptions.module;
-
-							if (id) moduleCache[id] = moduleObject;
-
-							tryRunOrWebpackError(
-								() =>
-									queried.call(
-										{
-											codeGenerationResult: new CodeGenerationResult(result),
-											moduleObject
-										},
-										{ __webpack_require__ }
-									),
-								"Compilation.hooks.executeModule"
-							);
-							moduleObject.loaded = true;
-							return moduleObject.exports;
-						};
+								const result = codegenResults.map[id]["build time"];
+								const moduleObject = execOptions.module;
+
+								if (id) moduleCache[id] = moduleObject;
+
+								tryRunOrWebpackError(
+									() =>
+										queried.call(
+											{
+												codeGenerationResult: new CodeGenerationResult(result),
+												moduleObject
+											},
+											{ __webpack_require__ }
+										),
+									"Compilation.hooks.executeModule"
+								);
+								moduleObject.loaded = true;
+								return moduleObject.exports;
+							};
 
-						const moduleCache: Record<string, any> = (__webpack_require__[
-							RuntimeGlobals.moduleCache.replace(
-								`${RuntimeGlobals.require}.`,
-								""
-							)
-						] = {});
-						const interceptModuleExecution: ((execOptions: any) => void)[] =
-							(__webpack_require__[
-								RuntimeGlobals.interceptModuleExecution.replace(
+							const moduleCache: Record<string, any> = (__webpack_require__[
+								RuntimeGlobals.moduleCache.replace(
 									`${RuntimeGlobals.require}.`,
 									""
 								)
-							] = []);
-
-						for (const runtimeModule of runtimeModules) {
-							__webpack_require__(runtimeModule);
-						}
+							] = {});
+							const interceptModuleExecution: ((execOptions: any) => void)[] =
+								(__webpack_require__[
+									RuntimeGlobals.interceptModuleExecution.replace(
+										`${RuntimeGlobals.require}.`,
+										""
+									)
+								] = []);
 
-						const executeResult = __webpack_require__(entry);
+							for (const runtimeModule of runtimeModules) {
+								__webpack_require__(runtimeModule);
+							}
 
-						that.deref()!.#moduleExecutionResultsMap.set(id, executeResult);
+							const executeResult = __webpack_require__(entry);
+							that.deref()!.#moduleExecutionResultsMap.set(id, executeResult);
+						} catch (e) {
+							that.deref()!.#moduleExecutionResultsMap.set(id, e);
+							throw e;
+						}
 					};
 				}
 			),

packages/rspack/src/loader-runner/index.ts

@@ -410,6 +410,42 @@ export async function runLoaders(
 		callback
 	) {
 		const options = userOptions ? userOptions : {};
+		const context = this;
+		function finalCallback(
+			onError: (err: Error) => void,
+			onDone: (res: any) => void
+		) {
+			return function (err?: Error, res?: any) {
+				if (err) {
+					onError(err);
+				} else {
+					for (const dep of res.buildDependencies) {
+						context.addBuildDependency(dep);
+					}
+					for (const dep of res.contextDependencies) {
+						context.addContextDependency(dep);
+					}
+					for (const dep of res.missingDependencies) {
+						context.addMissingDependency(dep);
+					}
+					for (const dep of res.fileDependencies) {
+						context.addDependency(dep);
+					}
+					if (res.cacheable === false) {
+						context.cacheable(false);
+					}
+
+					if (res.error) {
+						onError(
+							compiler.__internal__getModuleExecutionResult(res.id) ??
+								new Error(err)
+						);
+					} else {
+						onDone(compiler.__internal__getModuleExecutionResult(res.id));
+					}
+				}
+			};
+		}
 		if (!callback) {
 			return new Promise((resolve, reject) => {
 				compiler
@@ -419,81 +455,25 @@ export async function runLoaders(
 						options.layer,
 						options.publicPath,
 						options.baseUri,
-						context._module.moduleIdentifier,
+						context._module.identifier(),
 						loaderContext.context,
-						(err: Error, res: any) => {
-							if (err) reject(err);
-							else {
-								for (const dep of res.buildDependencies) {
-									this.addBuildDependency(dep);
-								}
-								for (const dep of res.contextDependencies) {
-									this.addContextDependency(dep);
-								}
-								for (const dep of res.missingDependencies) {
-									this.addMissingDependency(dep);
-								}
-								for (const dep of res.fileDependencies) {
-									this.addDependency(dep);
-								}
-								if (res.cacheable === false) {
-									this.cacheable(false);
-								}
-
-								if (res.error) {
-									reject(new Error(res.error));
-								} else {
-									resolve(
-										compiler.__internal__getModuleExecutionResult(res.id)
-									);
-								}
-							}
-						}
+						finalCallback(reject, resolve)
 					);
 			});
 		}
-		return compiler
-			._lastCompilation!.__internal_getInner()
-			.importModule(
-				request,
-				options.layer,
-				options.publicPath,
-				options.baseUri,
-				context._module.moduleIdentifier,
-				loaderContext.context,
-				(err: Error, res: any) => {
-					if (err) {
-						callback(err, undefined);
-					} else {
-						for (const dep of res.buildDependencies) {
-							this.addBuildDependency(dep);
-						}
-						for (const dep of res.contextDependencies) {
-							this.addContextDependency(dep);
-						}
-						for (const dep of res.missingDependencies) {
-							this.addMissingDependency(dep);
-						}
-						for (const dep of res.fileDependencies) {
-							this.addDependency(dep);
-						}
-						if (res.cacheable === false) {
-							this.cacheable(false);
-						}
-
-						if (res.error) {
-							callback(new Error(err), undefined);
-						} else {
-							callback(
-								undefined,
-								compiler.__internal__getModuleExecutionResult(res.id)
-							);
-						}
-					}
-				}
-			);
+		return compiler._lastCompilation!.__internal_getInner().importModule(
+			request,
+			options.layer,
+			options.publicPath,
+			options.baseUri,
+			context._module.identifier(),
+			loaderContext.context,
+			finalCallback(
+				err => callback(err),
+				res => callback(undefined, res)
+			)
+		);
 	} as LoaderContext["importModule"];
-
 	Object.defineProperty(loaderContext, "resource", {
 		enumerable: true,
 		get: () => {