fix: check for error before using other arguments

[nielsslot]

This PR contains a:

• bugfix
• new feature
• code refactor
• test update
• typo fix
• metadata update

Motivation / Use-Case

I was trying to get a Webpack build configuration to work, but I hit an error with mini-css-extract-plugin:

TypeError: Cannot read property 'getAssets' of undefined

The second line in the stack trace lead me to the Compiler.js file in the Webpack source code where it invokes a callback and passes only an error as argument. Looking at the code in mini-css-extract-plugin, it was trivial to see what wrong: compilation.getAssets() doesn't work when the compilation argument isn't passed and equals undefined.

By moving the check for errors to earlier in the callback I could actually see the original error with my Webpack build configuration and fix it from there. With this PR I hope to prevent someone else from experiencing the same problem.

[alexander-akait]

Please add failed case

[codecov]

Codecov Report

Merging #763 (2a030f7) into master (be1d6dc) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #763   +/-   ##
=======================================
  Coverage   89.31%   89.31%           
=======================================
  Files           6        6           
  Lines         805      805           
  Branches      247      247           
=======================================
  Hits          719      719           
  Misses         83       83           
  Partials        3        3           

Impacted Files	Coverage Δ
src/loader.js	90.53% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be1d6dc...2a030f7. Read the comment docs.

[Mesoptier]

@alexander-akait I'm not sure what more you really need here? Webpack can call this callback without the compilation argument. In which case trying to access properties on compilation will simply fail. This PR should obviously fix that problem.

[alexander-akait]

@Mesoptier in some cases you can have error and compilation, and I can show them, if it were so obvious I would have changed it a long time ago, which is why I am asking how the error was received, because most likely the problem needs to be fixed in the wrong place

[Mesoptier]

@alexander-akait Fair enough, but in that case should the code not check if compilation exists instead of assuming that it does?

My stack trace looks like this and is very similar to the one described in the first post of this PR:

UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'getAssets' of undefined
    at /code/node_modules/mini-css-extract-plugin/dist/loader.js:290:37
    at /code/node_modules/webpack/lib/Compiler.js:523:20
    at /code/node_modules/webpack/lib/Compiler.js:1099:25
    at /code/node_modules/webpack/lib/Compilation.js:2494:18
    at eval (eval at create (/code/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:11:1)
    at fn (/code/node_modules/webpack/lib/Compilation.js:427:17)
    at _next3 (eval at create (/code/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:9:1)
    at eval (eval at create (/code/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:25:1)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Looking at Compiler.js:523:20 tells me that this callback was in fact called with only the err argument.

I can't tell what caused this error to occur in the first place, because it was swallowed when TypeError: Cannot read property 'getAssets' of undefined was thrown.

[Mesoptier]

Actually the runAsChild callback is only called in two places:

1. if (err) return callback(err);
   https://github.com/webpack/webpack/blob/792c06a4bbeff3070d0d13acb2a951e4bec42e3c/lib/Compiler.js#L523
2. return callback(null, entries, compilation);
   https://github.com/webpack/webpack/blob/792c06a4bbeff3070d0d13acb2a951e4bec42e3c/lib/Compiler.js#L538

So I'm not sure what you're talking about when you say that it can get called with both an err and a compilation argument, @alexander-akait.

[alexander-akait]

@Mesoptier other plugins, we have many hooks, but looks like you have old webpack v5, am i right?

[Mesoptier]

@alexander-akait The stack trace I posted was using Webpack 5.39.0, so yeah it's a slightly older version. It doesn't look like the runAsChild or even the Compiler in general has changed code since that release though.

Do other plugins/hooks affect the internals of the runAsChild method? If not then I still don't see how this callback can be called with anything other than callback(err) or callback(null, entries, compilation), which would both be correctly handled by this PR.

[alexander-akait]

@Mesoptier by custom logic for running child compilation (multi threading), but here two notes:

• I am not against this change and most likely it is better for us to always throw an error for any error, beucase in 99% cases it is fatal
• I want to investigate why you get the problem (maybe we have problems in other places), so can you provide example of problem/stack trace or something else?

[Mesoptier]

@alexander-akait I haven't been able to obtain much more information about what caused the error to occur in the first place. It seems that it only occurs sporadically and only on my deploy server. So far the only information that I have is:

1. The error occurred for each of my .less entry points.
2. The error message is The "path" argument must be of type string. Received undefined'

I have now enabled stats.errorDetails: true, so if it happens again I'll hopefully get more details.

[alexander-akait]

@Mesoptier Maybe do you stacktrace? Can you add console.log inside mini-css-extract-plugin?

[Mesoptier]

@alexander-akait What I meant to say is that I'm not getting the error anymore. I've installed a patched version of mini-css-extract-plugin (basically this PR) that will log the original error if it ever occurs again. If I get more information, I will share it!

[alexander-akait]

Fixed here #772, big thanks for PR, release will be soon