Skip to content

Fix for #8398 - call failed-hook on compilation errors #8401

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

Merged
merged 1 commit into from
Dec 30, 2018

Conversation

shahkashani
Copy link
Contributor

@shahkashani shahkashani commented Nov 21, 2018

See: #8398

fixes #8398

The problem: you currently can't use hooks to determine whether a compilation has failed. This makes it hard to wrap compilers with custom error handling. In master, the only time hooks.failed is called is when compilation fails during watch-mode. This PR calls the hook for normal compilations also.

What kind of change does this PR introduce?

This PR ensures the failed-hooked is called whenever compilation fails.

The finalCallback (from compiler.run(finalCallback) fame) is called as a bail-out whenever there's an error. So adding the call to the failed-hook here makes sense because it minimizes the code changes.

Did you add tests for your changes?

Yes, added a test. To ensure this PR does the trick, either run the tests or the code below:

const webpack = require("webpack");

const compiler = webpack({
	bail: true,
	context: __dirname,
	mode: "production",
	entry: "./missing",
	output: {
		path: "/",
		filename: "bundle.js"
	}
});

compiler.hooks.failed.tap("CompilerTest", err => {
	console.log("I made it to Flavortown:\n", err);
});

compiler.run((err, stats) => {
	if (err || stats.hasError()) {
		console.log("Compilation failed.");
	}
});

Does this PR introduce a breaking change?

As far as I can tell, it should not break anything. That is, unless folks have existing code for the failed-hook that they never expect to be called, which seems unlikely.

What needs to be documented once your changes are merged?

The docs actually already state the failed-hook is called on failure, so nothing should need to be updated. This PR just makes the behavior match the docs.

@jsf-clabot

This comment has been minimized.

@shahkashani
Copy link
Contributor Author

Force-pushed to fix author name.

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@sokra sokra closed this Nov 21, 2018
@sokra sokra reopened this Nov 21, 2018
@sokra
Copy link
Member

sokra commented Nov 22, 2018

Please also sign the CLA.

@shahkashani
Copy link
Contributor Author

Will do! Might take a few days, I have to go through a lil’ process at work.

@shahkashani
Copy link
Contributor Author

Sorry for the delay. Signed 👍

@sokra sokra closed this Dec 4, 2018
@sokra sokra reopened this Dec 4, 2018
@ooflorent ooflorent closed this Dec 14, 2018
@ooflorent ooflorent reopened this Dec 14, 2018
@sokra sokra merged commit ccc7db7 into webpack:master Dec 30, 2018
@sokra
Copy link
Member

sokra commented Dec 30, 2018

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The failed hook is never called for normal compilations
5 participants