Always append the inner message to the ModuleBuildError message

[jennings]

What kind of change does this PR introduce?

improved error message

Did you add tests for your changes?

Yes

If relevant, link to documentation update:

N/A

Summary

When a loader throws an error with a stack trace, we weren't getting a helpful error message. It looks like it's because the error gets wrapped in a ModuleBuildError and the inner error's message isn't exposed.

I looked through the history of ModuleBuildError.cs and it seems like this behavior has been around since inception, so I'm not sure if the behavior is intentional or not.

Specifically, I'm using babel-loader with the plugin babel-plugin-undeclared-variables-check. This throws a BabelLoaderError back to webpack, where it's wrapped in a ModuleBuildError.

Does this PR introduce a breaking change?

Not to my knowledge

Other information

The error I get before this change:

ERROR in ./entry.js
Module build failed: Error
    at transpile (/Users/stephen/t/node_modules/babel-loader/lib/index.js:64:13)
    at Object.module.exports (/Users/stephen/t/node_modules/babel-loader/lib/index.js:174:20)

And after the change:

ERROR in ./entry.js
Module build failed: /Users/stephen/t/entry.js: Reference to undeclared variable "foo"

> 1 | foo()
    | ^
  2 |
Error
    at transpile (/Users/stephen/t/node_modules/babel-loader/lib/index.js:64:13)
    at Object.module.exports (/Users/stephen/t/node_modules/babel-loader/lib/index.js:174:20)

And the webpack.config.js:

var path = require('path')

module.exports = {
  entry: path.join(__dirname, 'entry.js'),

  output: {
    filename: 'output.js'
  },

  module: {
    rules: [
      {
        test: /\.js$/,
        use: {
          loader: 'babel-loader',
          options: {
            plugins: ['babel-plugin-undeclared-variables-check']
          }
        }
      }
    ]
  }
}

[webpack-bot]

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

[sokra]

I guess there could be a problem with the BabelLoaderError.

Could you try this?

 function BabelLoaderError(name, message, codeFrame, hideStack, error) {
   Error.call(this);
-  Error.captureStackTrace(this, BabelLoaderError);

   this.name = "BabelLoaderError";
   this.message = formatMessage(name, message, codeFrame);
   this.hideStack = hideStack;
   this.error = error;
+  Error.captureStackTrace(this, BabelLoaderError);
 }

captureStackTrace reads the current this.message and adds it to the stack trace (In the stack property the first line is usually the message), but message is written after the call. We did the same mistake in webpack.

[jennings]

Hmm, I think you're right. I should have checked the Node docs, they do say that the stack field is supposed to contain the message (I assumed it was just the stack trace).

I'll close this, thanks for your help.