Error handling improvements to esbuild plugin

[egnor]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and discussions and couldn’t find anything or linked relevant results below
• I made sure the docs are up to date
• I included tests (or that’s not needed)

Description of changes

This change improves(?) error conversion in the esbuild plugin, with no change to non-error function.

Previously, if a non-VFileMessage error was thrown by the MDX conversion process (eg. a simple nodejs runtime Error thrown by a plugin or the core), it would be wrapped as the cause in a VFileMessage with generic "Cannot process MDX file..." text, and re-thrown by the onload handler. This makes sense! However, when that message is converted in vfileMessageToEsbuild, the cause was completely ignored, so all information about that underlying error was lost, which makes debugging harder (especially debugging plugins).

Also previously, even when the original error thrown was a VFileMessage, if the optional place.start.offset (or place.offset) isn't defined, all place information was lost. This .offset field is optional (though set in most common paths).

This change revises things:

• In the onload handler, always throw a new VFileMessage with the original exception attached. Hoist location information to the outer exception, if the inner exception appears to be a VFileMessage itself. Include the inner exception's text rendering in the outer's text (after a colon).

• In vfileMessageToEsbuild, handle the case where offset information isn't present, working with whatever information is present.

The changes to the unit test are a decent way to see the effect of all this.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mdx	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 5, 2025 2:07pm

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (908ff45) to head (d3c901f).
Report is 86 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2595   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        21    -2     
  Lines         2693      2647   -46     
  Branches         2         2           
=========================================
- Hits          2693      2647   -46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[wooorm]

Suggested change

	`Cannot process MDX file with esbuild:\n ${error_}`, {
	'Cannot process MDX file with esbuild', {

The point of cause, I believe, is to be an alternative to copying it into the reason.
I’m fine with always making a new error, a vfile message.

[egnor]

Okay, I changed it to not concatenate the message at this point, and instead walk the cause chain and concatenate the text when creating the esbuild message.

In theory the cause chain is available in the esbuild message via the detail field, but, that's a very generic field and basically nothing seems to look at it-- certainly esbuild itself doesn't print it out when reporting errors.

[egnor]

Ready for another look!

[wooorm]

Are these lines because otherwise the “cause” is lost? That ESBuild doesn’t print them?

Is that something that ESbuild should fix? Upstream?

Or, if indeed needed here, perhaps you could use vfile-reporter I linked above, or, perhaps you could use inspect from util: https://nodejs.org/api/util.html#utilinspectobject-options. I believe that it serializes recursive causes

[egnor]

Okay yes this is the tricky bit.

In esbuild you can report errors by just throwing an exception, or by adding to a list of messages using their own Message class (which is what we're doing). That class includes a detail field which can be an exception but could also be anything else, and in any case doesn't get printed in esbuild's output, it seems to be for custom API use; it's not really comparable to cause.

Alternatives

• use vfile-reporter -- this does a fine job and does chase cause chains, unfortunately this wants an entire VFile, I don't see a way to format a single VFileMessage (though you could imagine exposing that)?
• use util.inspect -- this has a lovely formatter for Error objects, but even for subclasses like VFileMessage reverts to its default printer which just dumps the fields and does not print error messages or chase cause chains?
• just let the error get thrown rather than catching it and adding it to messages -- this does get handled by esbuild gracefully (including plugin attribution), but the extra location information in VFileMessage is lost
• add VFileMessage-shaped exceptions directly to messages (no wrapping), re-throw everything else, remove all cause-chain-processing -- this could work??

For now I simplified the cause processing to not try to walk the whole cause chain but just include the string representation of the cause if one exists.

[wooorm]

Good points, I appreciate that you came up with alternatives & discussed trade offs.

I could see an reportError in vfile-reporter, which can then be used here.

The no-wrapping idea could also perhaps work.

But this is probably fine to start with!

[wooorm]

Thanks! :)