Encore should fail with exit status code > 0 in case of unhandled promise rejection error

[ostrolucky]

Background: Google web fonts API changed its API and now one of the plugins fail.

// webpack.config.js
let Encore            = require('@symfony/webpack-encore');
let GoogleFontsPlugin = require("google-fonts-webpack-plugin");

Encore
    // the project directory where all compiled assets will be stored
    .setOutputPath('public/build/')

    // the public path used by the web server to access the previous directory
    .setPublicPath('/build')

    // will create public/build/app.js and public/build/app.scss
    .addEntry('app', './assets/js/app.js')

    .addPlugin(new GoogleFontsPlugin({
        fonts: [
            { family: "Open Sans", variants: ["400", "600"] }
        ],
        "path": "fonts/"
    }))
;

// export the final configuration
module.exports = Encore.getWebpackConfig();

$ /vagrant/backoffice-eh/node_modules/.bin/encore dev
Running webpack ...

(node:26399) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'map' of undefined
    at getVariantCss (/vagrant/backoffice-eh/node_modules/google-fonts-webpack-plugin/src/GoogleWebfonts.js:23:25)
    at variants.forEach.variant (/vagrant/backoffice-eh/node_modules/google-fonts-webpack-plugin/src/GoogleWebfonts.js:70:16)
    at Array.forEach (<anonymous>)
    at font.info.then.info (/vagrant/backoffice-eh/node_modules/google-fonts-webpack-plugin/src/GoogleWebfonts.js:67:14)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:26399) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:26399) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Done in 3.08s.
08:51 $ echo $?
0

This results in seemingly successful, but broken build, because manifest.js file is missing.

Plugin issue has been reported at https://github.com/SirPole/google-fonts-plugin/issues/31. However, I want to stress that I am not asking here to fix this particular issue with this plugin, I am requesting a process to fail with error exit status code in case of these errors, so that borked build is not deployed to production.

[stof]

Encore just calls the webpack runner. So that would be something to be reported to webpack. But I fear they won't want to fail on each unhandled rejection, as that would probably break watch mode (which must not stop). Instead, errors need to be handled by webpack.

The issue is that the google-fonts-plugin is buggy. webpack 4 has 2 async plugins API:

• compiler.hooks.*.tapAsync which expects a callback-based async API
• compiler.hooks.*.tapPromise which expects a promise-based async API google-fonts-plugin implements its code using an async function (so Promise-based) but registers it through tapAsync. This means that errors are not properly reported to webpack.

[ostrolucky]

Ok, how about if Encore would check if manifest.js file exists after webpack is done and fail the build then? This file is essential for encore bundle, otherwise server triggers 500. Then you don't need to tap into webpack hooks or rely on plugins doing that properly.

[stof]

Well, encore itself does not have any hook into when the webpack compilation is done, except by writing its own webpack plugins for that (Encore manages the entrypoint, but then delegates almost everything to the webpack runner or the webpack-dev-server runner). I don't think Encore should be responsible for ensuring that webpack plugins are implemented the right way (if you consider that plugin authors cannot be trusted for writing the right code, then such runtime checks should be requested to webpack itself, not to Encore).

Also, did the broken google-fonts-plugin actually prevent the generation of the manifest file ? If no, your suggestion would not have detected the issue either.

[ostrolucky]

Yes, it prevents generation of manifest file and also no other usual build steps like cache:clear check for existence of this file.