fix: buildEnd hook handle error (#902)

crates/rolldown/src/bundler.rs

@@ -80,20 +80,10 @@ impl Bundler {
   }
 
   async fn call_build_end_hook(&mut self, ret: &Result<ScanStageOutput>) -> Result<()> {
-    if let Ok(ret) = ret {
-      if let Some(error) = ret.errors.first() {
-        self
-          .plugin_driver
-          .build_end(Some(&HookBuildEndArgs {
-            // TODO(hyf0): 1.Need a better way to expose the error
-            error: error.to_string(),
-          }))
-          .await?;
-        return Ok(());
-      }
-    }
+    let args =
+      Self::normalize_error(ret, |ret| &ret.errors).map(|error| HookBuildEndArgs { error });
 
-    self.plugin_driver.build_end(None).await?;
+    self.plugin_driver.build_end(args.as_ref()).await?;
 
     Ok(())
   }
@@ -133,31 +123,28 @@ impl Bundler {
     let mut generate_stage =
       GenerateStage::new(&mut link_stage_output, &self.options, &self.plugin_driver);
 
-    let assets = {
+    let output = {
       let ret = generate_stage.generate().await;
 
-      self.call_render_error_hook(&ret, &link_stage_output.errors).await?;
+      if let Some(error) = Self::normalize_error(&ret, |ret| &ret.errors) {
+        self.plugin_driver.render_error(&HookRenderErrorArgs { error }).await?;
+      }
 
       ret?
     };
 
-    self.plugin_driver.generate_bundle(&assets, is_write).await?;
+    self.plugin_driver.generate_bundle(&output.assets, is_write).await?;
 
-    Ok(BundleOutput {
-      warnings: std::mem::take(&mut link_stage_output.warnings),
-      assets,
-      errors: std::mem::take(&mut link_stage_output.errors),
-    })
+    Ok(output)
   }
 
-  // The `render_error` hook should catch plugin errors.
-  async fn call_render_error_hook<T>(&self, ret: &Result<T>, errors: &[BuildError]) -> Result<()> {
-    if let Some(error) = ret
-      .as_ref()
-      .map_or_else(|error| Some(error.to_string()), |_| errors.first().map(ToString::to_string))
-    {
-      self.plugin_driver.render_error(&HookRenderErrorArgs { error }).await?;
-    }
-    Ok(())
+  fn normalize_error<T>(
+    ret: &Result<T>,
+    errors_fn: impl Fn(&T) -> &[BuildError],
+  ) -> Option<String> {
+    ret.as_ref().map_or_else(
+      |error| Some(error.to_string()),
+      |ret| errors_fn(ret).first().map(ToString::to_string),
+    )
   }
 }

crates/rolldown/src/module_loader/module_loader.rs

@@ -161,7 +161,7 @@ impl ModuleLoader {
   pub async fn fetch_all_modules(
     mut self,
     user_defined_entries: Vec<(Option<String>, ResolvedRequestInfo)>,
-  ) -> ModuleLoaderOutput {
+  ) -> anyhow::Result<ModuleLoaderOutput> {
     assert!(!self.input_options.input.is_empty(), "You must supply options.input to rolldown");
 
     let mut errors = vec![];
@@ -199,8 +199,6 @@ impl ModuleLoader {
 
     let mut runtime_brief: Option<RuntimeModuleBrief> = None;
 
-    let mut panic_errors = vec![];
-
     while self.remaining > 0 {
       let Some(msg) = self.rx.recv().await else {
         break;
@@ -254,14 +252,12 @@ impl ModuleLoader {
           errors.extend(e);
         }
         Msg::Panics(err) => {
-          panic_errors.push(err);
+          return Err(err);
         }
       }
       self.remaining -= 1;
     }
 
-    assert!(panic_errors.is_empty(), "Panics occurred during module loading: {panic_errors:?}");
-
     let modules: IndexVec<NormalModuleId, NormalModule> =
       self.intermediate_normal_modules.modules.into_iter().flatten().collect();
 
@@ -277,7 +273,7 @@ impl ModuleLoader {
       kind: EntryPointKind::DynamicImport,
     }));
 
-    ModuleLoaderOutput {
+    Ok(ModuleLoaderOutput {
       module_table: ModuleTable {
         normal_modules: modules,
         external_modules: self.external_modules,
@@ -288,6 +284,6 @@ impl ModuleLoader {
       runtime: runtime_brief.expect("Failed to find runtime module. This should not happen"),
       warnings: all_warnings,
       errors,
-    }
+    })
   }
 }

crates/rolldown/src/stages/generate_stage/mod.rs

@@ -21,7 +21,7 @@ use crate::{
     finalize_normal_module, is_in_rust_test_mode,
     render_chunks::render_chunks,
   },
-  SharedOptions,
+  BundleOutput, SharedOptions,
 };
 
 mod code_splitting;
@@ -43,7 +43,7 @@ impl<'a> GenerateStage<'a> {
   }
 
   #[tracing::instrument(skip_all)]
-  pub async fn generate(&mut self) -> Result<Vec<Output>> {
+  pub async fn generate(&mut self) -> Result<BundleOutput> {
     tracing::info!("Start bundle stage");
     let mut chunk_graph = self.generate_chunks();
 
@@ -145,7 +145,11 @@ impl<'a> GenerateStage<'a> {
 
     tracing::info!("rendered chunks");
 
-    Ok(assets)
+    Ok(BundleOutput {
+      assets,
+      warnings: std::mem::take(&mut self.link_output.warnings),
+      errors: std::mem::take(&mut self.link_output.errors),
+    })
   }
 
   fn generate_chunk_filenames(&self, chunk_graph: &mut ChunkGraph) {

crates/rolldown/src/stages/scan_stage.rs

@@ -70,7 +70,7 @@ impl ScanStage {
       warnings,
       errors,
       ast_table,
-    } = module_loader.fetch_all_modules(user_entries).await;
+    } = module_loader.fetch_all_modules(user_entries).await?;
     self.errors.extend(errors);
 
     tracing::debug!("Scan stage finished {module_table:#?}");

packages/rolldown/src/plugin/bindingify-build-hooks.ts

@@ -30,11 +30,7 @@ export function bindingifyBuildEnd(
   const [handler, _optionsIgnoredSofar] = normalizeHook(hook)
 
   return async (err) => {
-    try {
-      handler.call(null, err ?? undefined)
-    } catch (error) {
-      console.error(error)
-    }
+    handler.call(null, err ? new Error(err) : undefined)
   }
 }
 

packages/rolldown/src/plugin/index.ts

@@ -76,7 +76,8 @@ export interface Plugin {
     >
   >
 
-  buildEnd?: Hook<(this: null, err?: string) => MaybePromise<NullValue>>
+  buildEnd?: Hook<(this: null, err?: Error) => MaybePromise<NullValue>>
+
   // --- Generate hooks ---
 
   renderStart?: Hook<

packages/rolldown/tests/plugin/plugin.test.ts

@@ -1,4 +1,4 @@
-import { expect, test, vi } from 'vitest'
+import { expect, test, vi, describe } from 'vitest'
 import { rolldown, Plugin } from 'rolldown'
 
 async function buildWithPlugin(plugin: Plugin) {
@@ -31,3 +31,30 @@ test('Plugin renderError hook', async () => {
   })
   expect(renderErrorFn).toHaveBeenCalledTimes(1)
 })
+
+describe('Plugin buildEnd hook', async () => {
+  test('call buildEnd hook with error', async () => {
+    const buildEndFn = vi.fn()
+    await buildWithPlugin({
+      load() {
+        throw new Error('load error')
+      },
+      buildEnd: (error) => {
+        buildEndFn()
+        expect(error).toBeInstanceOf(Error)
+      },
+    })
+    expect(buildEndFn).toHaveBeenCalledTimes(1)
+  })
+
+  test('call buildEnd hook without error', async () => {
+    const buildEndFn = vi.fn()
+    await buildWithPlugin({
+      buildEnd: (error) => {
+        buildEndFn()
+        expect(error).toBeNull()
+      },
+    })
+    expect(buildEndFn).toHaveBeenCalledTimes(1)
+  })
+})