Problem with error handling

I ran into the following error during the initial compilation of an Angular 12 project, where simple-pug-loader is a dep of ngx-pug-builders:

Error: Module build failed (from ./node_modules/simple-pug-loader/index.js):
ng] ./node_modules/simple-pug-loader/index.js!./src/app/mods/comps/com.meta.create.comp.pug - Error: Module build failed (from ./node_modules/simple-pug-loader/index.js):
[ng] TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
[ng]     at validateString (internal/validators.js:120:11)
[ng]     at Object.normalize (path.js:1005:5)
[ng]     at Object.module.exports (/srv/project/client/node_modules/simple-pug-loader/index.js:104:38)
[ng]

Cursor 104:38 references the following line, in the catch statement:

loaderContext.addDependency(path.normalize(error.filename));

This is the same error reported by #11, however the explanation that closed that issue does not apply or is not accurate in my case. Actually I believe this is a bug related to #9.

For one thing, debugging proved the error was caused by the fact that 'error.filename' was undefined at this point, and that this is an additional error preventing an original error from being logged because the above statement is executed before the callback.

Given that there is already a 'filename" variable in scope, used for the pugOptions object, I went ahead and tried the following:

loaderContext.addDependency(path.normalize(filename));

This change resolved the above 'path' error and finally exposed the original error, which was related to an include using an absolute path, relative to the project root, without passing the basedir or root option with the rules config (again, during initial compilation), which I was able to subsequently correct.

This brings us back to the issue #9 attempted to fix, because while this resolves the error during initial compilation and allows the original (basedir related) error to finally be exposed, upon closer inspection it becomes clear when/why error.filename would be preferred, especially in the case of a syntax error within included files.

To illustrate, if we have parent.pug, which contains the include for partial.pug:

In the case of an error resolving the path of the include statement within parent.pug , error.filename is undefined while filename references parent.pug.
In the case of a syntax error within parent.pug, filename and error.filename reference parent.pug and so both would work just fine.
In the case of a syntax error within partial.pug, however, filename will reference parent.pug, while error.filename accurately references partial.pug.

In the last case, passing the filename variable would prevent watch from triggering upon changes to partial.pug, and of course when error.filename is passed everything works as expected.

This refactored catch statement appears to handle all of the above scenarios:

catch (error) {
    /*
     * Catch errors if needed
     */

    /*
     * Add the file where the error occurred as a dependency
     */
    //loaderContext.addDependency(path.normalize(error.filename));
    if (error.filename) {
      loaderContext.addDependency(path.normalize(error.filename));
    } else {
      loaderContext.addDependency(path.normalize(filename));
    }
    loaderContext.callback(error);
    return;
  }

Packages:

simple-pug-loader 0.2.1
webpack 5.50.0
ngx-pug-builders 12.0.0

Thanks.

spence-s / simple-pug-loader

Problem with error handling #12