A failing webpack build should fail the lektor build

[hacklschorsch]

We run lektor from GitLab CI/CD. It would be really nice if a failing (sass compile /) webpack build would bubble up to lektor to make it return 1 instead of 0 like its documentation says.

With the current behavior, without further precaution, a CI/CD pipeline would go on to deploy a broken website.

Example where I broke a bit of sass:

[...]
$ nix-shell -p nodejs --run "./result/bin/lektor build -f webpack --output-path lektor-result"
Starting webpack build
assets by status 106 KiB [cached] 5 assets
orphan modules 5.48 KiB [orphan] 7 modules
cacheable modules 7.11 KiB
  ./js/app.js + 7 modules 7.08 KiB [built] [code generated]
  ./sass/app.sass 39 bytes [built] [code generated] [1 error]
ERROR in ./sass/app.sass
Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
ModuleBuildError: Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
no mixin named desktop
        on line 227 of sass/components/global.sass
        from line 8 of sass/app.sass
>>   @include desktop;
   -----------^
    at processResult (/var/lib/private/gitlab-runner/builds/7GzFR-GM/0/privatestorage/privatestorageweb/webpack/node_modules/webpack/lib/NormalModule.js:764:19)
    at /var/lib/private/gitlab-runner/builds/7GzFR-GM/0/privatestorage/privatestorageweb/webpack/node_modules/webpack/lib/NormalModule.js:866:5
    at /var/lib/private/gitlab-runner/builds/7GzFR-GM/0/privatestorage/privatestorageweb/webpack/node_modules/loader-runner/lib/LoaderRunner.js:400:11
    at /var/lib/private/gitlab-runner/builds/7GzFR-GM/0/privatestorage/privatestorageweb/webpack/node_modules/loader-runner/lib/LoaderRunner.js:252:18
    at context.callback (/var/lib/private/gitlab-runner/builds/7GzFR-GM/0/privatestorage/privatestorageweb/webpack/node_modules/loader-runner/lib/LoaderRunner.js:124:13)
    at Object.loader (/var/lib/private/gitlab-runner/builds/7GzFR-GM/0/privatestorage/privatestorageweb/webpack/node_modules/sass-loader/dist/index.js:63:5)
 @ ./js/app.js 5:0-[35](https://whetstone.private.storage/privatestorage/privatestorageweb/-/jobs/20012#L35)
1 ERROR in child compilations (Use 'stats.children: true' resp. '--stats-children' for more details)
webpack 5.86.0 compiled with 2 errors in 1102 ms
Webpack build finished
U index.html
U 404.html
[...]
U static/img/Illustrations/ProgressComplete.png
Finished build in 7.77 sec
Started prune
Finished prune in 0.01 sec
Uploading artifacts for successful job
00:02
Uploading artifacts...
[...]

[dairiki]

I agree.

Would simply raising (an uncaught) subprocess.CalledProcessError when any of the forked subprocesses (npm, yarn, and/or webpack) fail be sufficient? Or should we report the error in some prettier, more user-friendly fashion before crapping out?

(PRs welcome.)

This might also be the time to upgrade the code to modern python — e.g. I'm not sure we need lektor.utils.portable_popen anymore, just use subprocess.run instead.

[hacklschorsch]

Thank you for your prompt reply!

Would simply raising (an uncaught) subprocess.CalledProcessError when any of the forked subprocesses (npm, yarn, and/or webpack) fail be sufficient?

Sure!

Especially since it looks like the invoked tools write their error to the terminal already.