Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix handling of multiple compilation errors #1011

Merged
merged 1 commit into from
Nov 23, 2021
Merged

Fix handling of multiple compilation errors #1011

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 17 additions & 4 deletions lib/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function webpackCompile(config, logStats, ServerlessError) {
_.forEach(stats, compileStats => {
logStats(compileStats);
if (compileStats.hasErrors()) {
throw new ServerlessError('Webpack compilation error, see stats above');
throw _.assign(new ServerlessError('Webpack compilation error, see stats above'), { stats: compileStats });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not actually a change request, but rather grabbing the chance to discuss with people who cares about the project.

When I am experimenting the reverting of MultiCompile/ combined entries approach, I can't help but think of a complete rewrite of this project with TypeScript. The monkey patching approach brought down from Serverless V1 makes debugging and error tracing a bit tricky in many cases.

The effort is not small though, do you think it's worth doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vicary thanks for comment, but do you see this discussion specific to this contribution?

This PR is about grouping all compilation errors into one error, and not maintaining a situation where first one is reported, and others jump out aside (after Framework reports that command processing is finalized)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more a global discussion not specific to your contribution

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing that you're adding stats to the ServerlessError object, I just expanded the topic a bit.

But definitely not a blocker for this issue, just general discussion.

}
});

Expand All @@ -99,9 +99,22 @@ function webpackCompile(config, logStats, ServerlessError) {
}

function webpackConcurrentCompile(configs, logStats, concurrency, ServerlessError) {
return BbPromise.map(configs, config => webpackCompile(config, logStats, ServerlessError), { concurrency }).then(
stats => _.flatten(stats)
);
const errors = [];
return BbPromise.map(
configs,
config =>
webpackCompile(config, logStats, ServerlessError).catch(error => {
errors.push(error);
return error.stats;
}),
{ concurrency }
).then(stats => {
if (errors.length) {
if (errors.length === 1) throw errors[0];
medikoo marked this conversation as resolved.
Show resolved Hide resolved
throw new ServerlessError('Webpack compilation errors, see stats above');
}
return _.flatten(stats);
});
}

module.exports = {
Expand Down