feat(5858): make closeBundle hook receive the last error (#5867)

* feat(5858): make closeBundle hook receive the last error

* keep the error chain

* dry

* Revert "dry"

This reverts commit 6a3d70f4fe7baf75507911ef6de6dfcf374b4f9b.

* watch files (without tests for now)

* compound error

* oops, watchFiles

* docs: almost forgot

* Update docs/plugin-development/index.md

* Update docs/plugin-development/index.md

* indeed it works

Author: GauBen

docs/plugin-development/index.md

@@ -877,14 +877,16 @@ Cf. [`output.banner/output.footer`](../configuration-options/index.md#output-ban
 
 |  |  |
 | --: | :-- |
-| Type: | `closeBundle: () => Promise<void> \| void` |
+| Type: | `closeBundle: (error?: Error) => Promise<void> \| void` |
 | Kind: | async, parallel |
 | Previous: | [`buildEnd`](#buildend) if there was a build error, otherwise when [`bundle.close()`](../javascript-api/index.md#rollup-rollup) is called, in which case this would be the last hook to be triggered |
 
 Can be used to clean up any external service that may be running. Rollup's CLI will make sure this hook is called after each run, but it is the responsibility of users of the JavaScript API to manually call `bundle.close()` once they are done generating bundles. For that reason, any plugin relying on this feature should carefully mention this in its documentation.
 
 If a plugin wants to retain resources across builds in watch mode, they can check for [`this.meta.watchMode`](#this-meta) in this hook and perform the necessary cleanup for watch mode in [`closeWatcher`](#closewatcher).
 
+If an error occurs during build or the `buildEnd` hook, it is passed to this hook as first argument.
+
 ### footer
 
 |  |  |

src/rollup/rollup.ts

@@ -9,6 +9,7 @@ import { LOGLEVEL_DEBUG, LOGLEVEL_INFO, LOGLEVEL_WARN } from '../utils/logging';
 import { getLogHandler } from '../utils/logHandler';
 import {
 	error,
+	getRollupError,
 	logAlreadyClosed,
 	logCannotEmitFromOptionsHook,
 	logMissingFileOrDirOption,
@@ -79,11 +80,26 @@ export async function rollupInternal(
 			if (watchFiles.length > 0) {
 				error_.watchFiles = watchFiles;
 			}
-			await graph.pluginDriver.hookParallel('buildEnd', [error_]);
-			await graph.pluginDriver.hookParallel('closeBundle', []);
+			try {
+				await graph.pluginDriver.hookParallel('buildEnd', [error_]);
+			} catch (buildEndError: any) {
+				// Create a compound error object to include both errors, based on the original error
+				const compoundError = getRollupError({
+					...error_,
+					message: `There was an error during the build:\n  ${error_.message}\nAdditionally, handling the error in the 'buildEnd' hook caused the following error:\n  ${buildEndError.message}`
+				});
+				await graph.pluginDriver.hookParallel('closeBundle', [compoundError]);
+				throw compoundError;
+			}
+			await graph.pluginDriver.hookParallel('closeBundle', [error_]);
 			throw error_;
 		}
-		await graph.pluginDriver.hookParallel('buildEnd', []);
+		try {
+			await graph.pluginDriver.hookParallel('buildEnd', []);
+		} catch (buildEndError: any) {
+			await graph.pluginDriver.hookParallel('closeBundle', [buildEndError]);
+			throw buildEndError;
+		}
 	});
 
 	timeEnd('BUILD', 1);

src/rollup/types.d.ts

@@ -404,7 +404,7 @@ export interface FunctionPluginHooks {
 	augmentChunkHash: (this: PluginContext, chunk: RenderedChunk) => string | void;
 	buildEnd: (this: PluginContext, error?: Error) => void;
 	buildStart: (this: PluginContext, options: NormalizedInputOptions) => void;
-	closeBundle: (this: PluginContext) => void;
+	closeBundle: (this: PluginContext, error?: Error) => void;
 	closeWatcher: (this: PluginContext) => void;
 	generateBundle: (
 		this: PluginContext,

test/hooks/index.js

@@ -1032,6 +1032,99 @@ describe('hooks', () => {
 			});
 	});
 
+	it('passes build error to the closeBundle hook', () => {
+		let buildError;
+		return rollup
+			.rollup({
+				input: 'input',
+				plugins: [
+					loader({ input: 'some broken code' }),
+					{
+						closeBundle(error) {
+							buildError = error;
+						}
+					}
+				]
+			})
+			.then(bundle => bundle.close())
+			.catch(error => {
+				assert.strictEqual(
+					error.message,
+					`input (1:5): Expected ';', '}' or <eof> (Note that you need plugins to import files that are not JavaScript)`
+				);
+			})
+			.then(() => {
+				assert.ok(buildError);
+			});
+	});
+
+	it('passes buildEnd error to the closeBundle hook', () => {
+		let buildEndError;
+		return rollup
+			.rollup({
+				input: 'input',
+				plugins: [
+					loader({ input: 'console.log(42);' }),
+					{
+						buildEnd() {
+							this.error('build end error');
+						},
+						closeBundle(error) {
+							buildEndError = error;
+						}
+					}
+				]
+			})
+			.then(bundle => bundle.close())
+			.catch(error => {
+				assert.strictEqual(error.message, '[plugin at position 2] build end error');
+			})
+			.then(() => {
+				assert.ok(buildEndError);
+			});
+	});
+
+	it('merges build and buildEnd errors', () => {
+		let buildEndError;
+		return rollup
+			.rollup({
+				input: 'input',
+				plugins: [
+					loader({ input: 'some broken code' }),
+					{
+						buildEnd() {
+							this.error('build end error');
+						},
+						closeBundle(error) {
+							buildEndError = error;
+						}
+					}
+				]
+			})
+			.then(bundle => bundle.close())
+			.catch(error => {
+				// Ensure error correctly copies the original build error
+				assert.strictEqual(error.cause.message, "Expected ';', '}' or <eof>");
+				assert.strictEqual(error.code, 'PARSE_ERROR');
+				assert.strictEqual(error.frame, '1: some broken code\n        ^');
+				assert.strictEqual(error.id, 'input');
+				assert.deepStrictEqual(error.loc, { file: 'input', line: 1, column: 5 });
+				assert.strictEqual(
+					error.message,
+					'There was an error during the build:\n' +
+						"  input (1:5): Expected ';', '}' or <eof> (Note that you need plugins to import files that are not JavaScript)\n" +
+						"Additionally, handling the error in the 'buildEnd' hook caused the following error:\n" +
+						'  [plugin at position 2] build end error'
+				);
+				assert.strictEqual(error.name, 'RollupError');
+				assert.strictEqual(error.pos, 5);
+				assert.ok(error.stack.startsWith(`${error.name}: ${error.message}\n`));
+			})
+			.then(() => {
+				assert.ok(buildEndError);
+			});
+	});
+
 	it('supports disabling sanitization for in-memory / in-browser / non-fs builds', () =>
 		rollup
 			.rollup({