Wrap errors in an `AggregateError`

[remcohaszing]

Wrap errors in an `AggregateError`

[wooorm]

Did you actually run into this? What did you see, what did you expect?

If only one error is thrown, perhaps no AggregateError is needed.

🤔🤔🤔 Well, one error is thrown, so you are saying AggregateError is not needed? 🤔🤔🤔

[remcohaszing]

Did you actually run into this? What did you see, what did you expect?

It took me a minute to understand what’s going on with https://github.com/orgs/remarkjs/discussions/1333. I think an AggregateError would have helped both me and the user understand what’s going wrong.

If only one error is thrown, perhaps no AggregateError is needed.

🤔🤔🤔 Well, one error is thrown, so you are saying AggregateError is not needed? 🤔🤔🤔

Multiple errors are thrown and caught. Currently only one is preserved and rethrown.

If from.length === 1 and plugin is falsy, then only one error is ever thrown. In that situation I’m not sure if it’s useful to wrap it in an AggregateError. There’s something to say for either IMO.

---

For reference, based on this script:

try {
  throw new Error('a')
} catch(a) {
  try {
    throw new Error('b')
  } catch(b) {
    throw new AggregateError([a, b], 'Oh no!')
  }
}

This is what an AggregateError looks like in Node.js:

AggregateError: Oh no!
    at file:///home/remco/script.js:7:11
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5) {
  [errors]: [
    Error: a
        at file:///home/remco/script.js:2:9
        at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
        at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
        at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5),
    Error: b
        at file:///home/remco/script.js:5:11
        at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
        at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
        at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5)
  ]
}

[wooorm]

It took me a minute to understand what’s going on with github.com/orgs/remarkjs/discussions/1333

Perhaps the tools between load-plugin and the user should throw a better error? Wrap it, and expose the currently thrown error as a cause?

Or load-plugin should throw a better error, with a cause?

Or, you could infer from the error that this was running in Electron and hence the baked in Node packages were checked?

How would you imagine that error to be improved?

Multiple errors are thrown and caught. Currently only one is preserved and rethrown.

What is a bit weird about this is that we do not know which errors are correct and which not.
Or, more correctly, we know that up to one error is correct, and all the others are incorrect: e.g., someone is supposed to have say remark-gfm in one particular place, probably not in the other places?

[remcohaszing]

Why aren’t all errors correct? That’s why I propose using AggregateError. It’s basically an error with multiple causes.

So it would look something like this:

- let lastError
+ const errors = []
  …
- lastError = error
+ errors.push(error)
  …
- throw lastError
+ throw new AggregateError(errors, 'Failed to resolve ' + name)

[wooorm]

Why aren’t all errors correct?

This is a very low level tool. It can be called with 'gfm' and 'remark' and './path/to/folder' and './path/to/cwd' and './path/to/global/modules' and './electron/baked/in/modules'. Then it would aggregate 8 errors.

It would generate those 8 errors too when the user forgot to run npm i and so './path/to/folder/node_modules/remark-gfm' was missing.

Another reason this could fail, is when Node is already trying to read X files on some low-resource computer. 7 errors would be about remark-gfm and gfm being missing. One about not being to read some file because ERR_BUSY.
Or similarly, someone was running npm install in parallel which changed the format of node_modules, previously files existed but not anymore.
The user is not helped by 8 errors.

The reason you raise this issue for a confusing error message, shows that one of the error messages is confusing.

---

Can you also consider the other questions? I think those are important. I would imagine that you were helped more by a better error message in vscode-remark. Or even a better error message here. Compared to 8 errors.

[remcohaszing]

The user is not helped by 8 errors.

The reason you raise this issue for a confusing error message, shows that one of the error messages is confusing.

I don’t understand. What’s confusing here IMO is that one of these 8 errors needs to be resolved. Which one that is, depends on a context that load-plugin doesn’t have. Each individual error message specifically is fine, but the user only gets to know the last error. Aggregating the errors into one solves exactly that. I don’t see why the user isn’t helped by showing all errors instead of just the last one.

Are you worried having 8 stack traces is overwhelming? Another solution would be to throw a new error, whose error message is all collected error messages joined by a newline.

---

Perhaps the tools between load-plugin and the user should throw a better error?

Perhaps, but I really do think the individual errors are fine. The problem is that only one error is surfaced, while another error may be more helpful depending on context.

Wrap it, and expose the currently thrown error as a cause?

Sure, load-plugin probably swallows the relevant error. Or perhaps there are multiple errors that can be acted upon. For example installing remark globally or in the project locally can both be viable solutions.

Or load-plugin should throw a better error, with a cause?

Sure, but which error is the cause? This is what AggregateError is for.

Can you also consider the other questions?

I tried, but I really keep circling back to the same problem. All errors are caught, but only one error is thrown. All others are swallowed.

[wooorm]

I don’t see why the user isn’t helped by showing all errors instead of just the last one.
Are you worried having 8 stack traces is overwhelming?

Yes, keeping that vscode-remark user in mind, who was confused by that one error stack, now imagine how 4-8 times that error stack would look to them.

Your “Which one that is, depends on a context that load-plugin doesn’t have.” is indeed a good point, and why I believe that that user is helped more by an improved error above load-plugin.
That’s my previous point “Perhaps the tools between load-plugin and the user should throw a better error?”.
I don’t think we can help that vscode-remark user in load-plugin.

I do agree that the error here could perhaps be improved, but I think improving goes further than just showing 4-8 errors.
That’s my previous point “Or load-plugin should throw a better error, with a cause?”.
Thinking further on this, most of the errors caught here will likely and expectedly be that remark-gfm is not installed in paths X, Y, and Z, and gfm is not installed in paths X, Y, and Z either.
There could also be arbitrary errors, but this is not the common case.
So, we could detect if all errors are about gfm/remark-gfm missing in X, Y, Z, and then show a pretty error for that, or alternatively show an (aggregated) error for the arbitrary non-missing errors we get.

[remcohaszing]

I don’t see why the user isn’t helped by showing all errors instead of just the last one.
Are you worried having 8 stack traces is overwhelming?

Yes, keeping that vscode-remark user in mind, who was confused by that one error stack, now imagine how 4-8 times that error stack would look to them.

Ok, this makes sense.

Your “Which one that is, depends on a context that load-plugin doesn’t have.” is indeed a good point, and why I believe that that user is helped more by an improved error above load-plugin. That’s my previous point “Perhaps the tools between load-plugin and the user should throw a better error?”. I don’t think we can help that vscode-remark user in load-plugin.

The problem is that the tool above load-plugin doesn’t have this context either. load-plugin determines whether or not it should look for electron or nvm related folders. And in this particular case whether to install remark locally or in the project is up to the user.

I do agree that the error here could perhaps be improved, but I think improving goes further than just showing 4-8 errors. That’s my previous point “Or load-plugin should throw a better error, with a cause?”. Thinking further on this, most of the errors caught here will likely and expectedly be that remark-gfm is not installed in paths X, Y, and Z, and gfm is not installed in paths X, Y, and Z either. There could also be arbitrary errors, but this is not the common case. So, we could detect if all errors are about gfm/remark-gfm missing in X, Y, Z, and then show a pretty error for that, or alternatively show an (aggregated) error for the arbitrary non-missing errors we get.

I’m not a fan of phrasing “with a cause”, because there are multiple causes.

I think we can get quite far by showing an error message like (this can probably be improved):

let errorMessage = 'Failed to load ' + name
if (plugin) {
  errorMessage += ' or ' + plugin
}
errorMessage += 'from ' + from.join(' or ')

throw new Error(errorMessage)

IMO it could still be an AggregateError though. The tool implementing AggregateError can choose whether to show the error message, stack trace, or complete error context. For reference, this script:

import { inspect } from 'node:util'

try {
  try {
    throw new Error('a')
  } catch (a) {
    try {
      throw new Error('b')
    } catch (b) {
      throw new AggregateError([a, b], 'Oh no!')
    }
  }
} catch (error) {
  console.log(error.message)
  console.log()
  console.log(error.stack)
  console.log()
  console.log(inspect(error))
}

yields:

Oh no!

AggregateError: Oh no!
    at file:///home/remco/script.mjs:10:13
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5)

AggregateError: Oh no!
    at file:///home/remco/script.mjs:10:13
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5) {
  [errors]: [
    Error: a
        at file:///home/remco/script.mjs:5:11
        at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
        at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
        at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5),
    Error: b
        at file:///home/remco/script.mjs:8:13
        at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
        at async ModuleLoader.import (node:internal/modules/esm/loader:475:24)
        at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5)
  ]
}

In the case of vscode-remark, I imagine it’s useful to show the error message to the user, and log the full AggregateError including all stack traces in the logs for troubleshooting purposes.

[wooorm]

There are many possible errors, not just errors for unfound packages. Your pseudocode is a fine idea for the “So, we could detect if all errors are about gfm/remark-gfm missing in X, Y, Z, and then show a pretty error for that” part but it is not good for other arbitrary errors (“show an (aggregated) error for the arbitrary non-missing errors we get.”).

In the case of vscode-remark, I imagine it’s useful to show the error message to the user, and log the full AggregateError including all stack traces in the logs for troubleshooting purposes.

I really doubt that those 4 error stacks help that user. I really think the only good fix for that user is in https://github.com/unifiedjs/unified-language-server/blob/73aa2bd151228ca5971befe4c8d76746e782e95c/lib/index.js#L198-L201.

[remcohaszing]

There are many possible errors, not just errors for unfound packages. Your pseudocode is a fine idea for the “So, we could detect if all errors are about gfm/remark-gfm missing in X, Y, Z, and then show a pretty error for that” part but it is not good for other arbitrary errors (“show an (aggregated) error for the arbitrary non-missing errors we get.”).

Why is it not good for arbritrary errors? My example error message stated it failed to load gfm/gfm-remark in X, Y, Z. That seems correct to me. It doesn’t say they are missing.

In the case of vscode-remark, I imagine it’s useful to show the error message to the user, and log the full AggregateError including all stack traces in the logs for troubleshooting purposes.

I really doubt that those 4 error stacks help that user. I really think the only good fix for that user is in https://github.com/unifiedjs/unified-language-server/blob/73aa2bd151228ca5971befe4c8d76746e782e95c/lib/index.js#L198-L201.

I agree those stack traces are probably not helpful for the user, only overwhelming even. However, those stack traces are useful when troubleshooting. If they are in the logs, the user is not bothered with them, but we can ask them to look it up if we need the extra information.

I’m currently investigating mdx-js/mdx-analyzer#442. There appears to be some lower level issue. The only way to investigate this is to use the debugger to inspect the errors that are currently being swallowed by load-plugin.

[wooorm]

I’d love for you to try to solve this issue here inside your node_modules for 442, in 2 ways: one with just an aggregate error, and another with an improved error message that shows which paths were checked. That way, we can actually compare the different approaches.

Why is it not good for arbritrary errors? My example error message stated it failed to load gfm/gfm-remark in X, Y, Z. That seems correct to me. It doesn’t say they are missing.

These are talking about errors because something was not found in a particular path (the x, y, z). There are also many other reasons for errors to occur with import-meta-resolve, and then actually importing files. I’d imagine there are >=100 different errors possible.

[remcohaszing]

I resolved the issue. These are basically

• The user reports mdx-js/mdx-analyzer#442
• Using the test fixtures I’m unable to reproduce the issue. I asked for a reproduction.
• The user provides a reproduction repository.
• After cloning and installing the repository, I see the following error:
  
• After opening the MDX extension output, I see the following error:

  Error: Cannot find package 'remark-frontmatter' imported from /home/remco/.local/lib/node_modules
      at new n (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:858:18648)
      at Sq (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language- server.js:860:7076)
      at Xle (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:860:7581)
      at Eq (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:860:8808)
      at Gv (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:860:8998)
      at Lq (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:860:10157)
      at async Kv (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:860:9400)
      at async Promise.all (index 0)
      at async s (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:865:18826)
      at async sae (/home/remco/.vscode/extensions/unifiedjs.vscode-mdx-1.8.6/out/language-server.js:96:5817) {
    code: 'ERR_MODULE_NOT_FOUND'
  }
  

• That’s weird, this output doesn’t match the stack trace the user showed.
• Let’s use the debugging setup in the VSCode extension repository.

  Error: Cannot find package 'remark-frontmatter' imported from /home/remco/.local/lib/node_modules
      at new NodeError (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:87524:5)
      at packageResolve (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:88465:9)
      at moduleResolve (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:88505:18)
      at defaultResolve (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:88596:15)
      at resolve (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:88613:12)
      at resolvePlugin (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:88692:14)
      at async loadPlugin (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:88637:16)
      at async Promise.all (index 0)
      at async getLanguagePlugins (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:91289:17)
      at async createTypeScriptServerProject (/home/remco/Projects/mdx-analyzer/packages/vscode-mdx/out/language-server.js:26723:31) {
    code: 'ERR_MODULE_NOT_FOUND'
  }
  

• Those unminified names are better, but still not great.
• Let’s enable source maps (3104d7c45c066c3a19e746fa8cbb47426d8b1867).

  Error: Cannot find package 'remark-frontmatter' imported from /home/remco/.local/lib/node_modules
      at __node_internal_ (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/errors.js:436:11)
      at NodeError (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/errors.js:380:5)
      at packageResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1035:9)
      at moduleResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1113:18)
      at defaultResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1289:15)
      at resolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/index.js:32:12)
      at resolvePlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:185:14)
      at loadPlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:82:16)
      at async Promise.all (index 0)
      at getLanguagePlugins (/home/remco/Projects/mdx-analyzer/packages/language-server/index.js:103:17)
      at createTypeScriptServerProject (/home/remco/Projects/mdx-analyzer/node_modules/@volar/language-server/lib/project/typescriptProject.js:51:29) {
    code: 'ERR_MODULE_NOT_FOUND'
  }
  

• That’s much better already, but it’s still not the error we’re looking for.
• Let’s use an patch load-plugin to use an AggregateError and a dedicated error message as per #21 (comment)

  AggregateError: Failed to load remark-frontmatter from file:///home/remco/Projects/mdx-ext-debug or file:///home/remco/.local/lib/node_modules
      at resolvePlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:198:9)
      at loadPlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:82:16)
      at async Promise.all (index 0)
      at getLanguagePlugins (/home/remco/Projects/mdx-analyzer/packages/language-server/index.js:103:17)
      at createTypeScriptServerProject (/home/remco/Projects/mdx-analyzer/node_modules/@volar/language-server/lib/project/typescriptProject.js:51:29) {
    [errors]: [
      Error: Cannot find package 'remark-frontmatter' imported from /home/remco/Projects/mdx-ext-debug
          at __node_internal_ (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/errors.js:436:11)
          at NodeError (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/errors.js:380:5)
          at packageResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1035:9)
          at moduleResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1113:18)
          at defaultResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1289:15)
          at resolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/index.js:32:12)
          at resolvePlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:186:14)
          at loadPlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:82:16)
          at async Promise.all (index 0)
          at getLanguagePlugins (/home/remco/Projects/mdx-analyzer/packages/language-server/index.js:103:17)
          at createTypeScriptServerProject (/home/remco/Projects/mdx-analyzer/node_modules/@volar/language-server/lib/project/typescriptProject.js:51:29) {
        code: 'ERR_MODULE_NOT_FOUND'
      },
      Error: Cannot find package 'remark-frontmatter' imported from /home/remco/.local/lib/node_modules
          at __node_internal_ (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/errors.js:436:11)
          at NodeError (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/errors.js:380:5)
          at packageResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1035:9)
          at moduleResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1113:18)
          at defaultResolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/lib/resolve.js:1289:15)
          at resolve (/home/remco/Projects/mdx-analyzer/node_modules/import-meta-resolve/index.js:32:12)
          at resolvePlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:186:14)
          at loadPlugin (/home/remco/Projects/mdx-analyzer/node_modules/load-plugin/lib/index.js:82:16)
          at async Promise.all (index 0)
          at getLanguagePlugins (/home/remco/Projects/mdx-analyzer/packages/language-server/index.js:103:17)
          at createTypeScriptServerProject (/home/remco/Projects/mdx-analyzer/node_modules/@volar/language-server/lib/project/typescriptProject.js:51:29) {
        code: 'ERR_MODULE_NOT_FOUND'
      }
    ]
  }
  

• Now this is useful.
• From just the error message I can see:
  • It only tries remark-frontmatter, not for example frontmatter or remark-remark-frontmatter.
  • It tries to load remark-frontmatter from both the project locally, and from the global installation.
  • I know VSCode uses Electron, and load-plugin has some logic regarding Electron and nvm. Neither of those is checked. (Which is fine IMO.)
  • The path to resolve globally has node_modules appended, but the path to resolve locally does not. This is suspicious.
• From the multiple stack traces I can see the following:
  • Resolving the module locally or globally throws the exact same error trace.
  • At least neither error is caused by some weird unexpected file system / memory / CPU issues. Both look like Node.js userland issues.
  • The issue comes from import-meta-resolve.

After this I narrowed it down by debugging and tweaking a small Node.js script. It turns out import-meta-resolve uses URLs relative to the input. So both the local URL provided by mdx-analyzer and the global URL provided by load-plugin are wrong. The URLs should end with /. Trying to resolve ./node_modules/remark-frontmatter/package.json, at some point the URL constructor will strip mdx-ext-debug from file:///home/remco/Projects/mdx-ext-debug and resolve the URL to file:///home/remco/Projects/node_modules/remark-frontmatter/package.json. Likewise, at some point the URL constructor will strip node_modules from file:///home/remco/.local/lib/node_modules and resolve the URL to file:///home/remco/.local/lib/node_modules/remark-frontmatter/package.json. So resolving globally kind of works by accident.

In the end the fix was to add a trailing slash to the URL passed into load-plugin. (0e2e0e7f373eda12a38af0a7ab3b3f816efffdd4)

[wooorm]

Thank you!

Let’s use an patch load-plugin to use an AggregateError and a dedicated error message as per

Which patch exactly?

From just the error message I can see: […]

So what I am wondering is whether all that can be inferred by an improved error here.
Have you checked that? Would you be interested in investigating that?

Neither of those is checked. (Which is fine IMO.)

IIRC you don‘t use nvm?

Now this is useful.

I don’t think the user would have been helped by showing them this aggregated error.
In unified-engine I wrap errors from load-plugin.
Have you considered wrapping errors from load-plugin in mdx-analyzer?

It turns out import-meta-resolve uses URLs relative to the input

Indeed, import.meta.resolve (and window.location and URL) works from files. The old CJS require.resolve works from folders

In the end the fix was to add a trailing slash to the URL passed into load-plugin.

That cwd variable seems to refer to path.dirname(tsconfig), perhaps instead pass pathToFileURL(tsconfig)?