Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Zod error messages and user config error messages #12634

Merged
merged 13 commits into from
Dec 5, 2024

Conversation

delucis
Copy link
Member

@delucis delucis commented Dec 4, 2024

Changes

  • Upstreams improvements to the Zod error map that we made in Starlight. We initially copied Astro’s error map and then iterated on it. It’s still similar, just with better handling for complex types like unions, and more consistent formatting (e.g. if you look at the changed tests you’ll see we’d report errors where some stuff was wrapped with backticks and other stuff wasn’t within the same error message and that that is now fixed).

  • Updates error handling for user config parse errors to use the error map. With the v5 release, we’re seeing a fair amount of the rather confusing output Invalid input error when people leave output: "hybrid" in their config and this aims to improve that.

    • This also updates the styling for these messages as they can now contain a bit more useful information. Open to feedback/suggestions on the design there. It reuses the Markdown formatter we already use for error rendering and adds some additional details for this specific context.
  • Fixes a broken docs link in one of our error messages.

Example

Example error message with this change when adding output: 'hybrid', experimental: { contentLayer: true } to an astro.config file:

Currently

Error message. The text is cramped together and one error simply reads "output Invalid input"

After this PR

Error message. The text for each issue is spaced out and the same error now reads output: Did not match union.  Expected "static" | "server", received "hybrid"

Testing

  • I’ve updated our existing error message tests to test for the updated message format
  • I manually ran astro dev in the blog example using invalid configurations to test output.

Docs

n/a — error message improvements only

Copy link

changeset-bot bot commented Dec 4, 2024

🦋 Changeset detected

Latest commit: 2170904

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Dec 4, 2024
Copy link

codspeed-hq bot commented Dec 4, 2024

CodSpeed Performance Report

Merging #12634 will not alter performance

Comparing chris/better-error-messages (2170904) with main (22e405a)

Summary

✅ 4 untouched benchmarks

@delucis delucis marked this pull request as ready for review December 4, 2024 22:53
@delucis delucis changed the title Improve Zod error messages Improve Zod error messages and user config error messages Dec 4, 2024
@florian-lefebvre
Copy link
Member

I think the error map is exported and used in astro:db, do you think you could import it and use it as is in Starlight?

@delucis
Copy link
Member Author

delucis commented Dec 5, 2024

I think the error map is exported and used in astro:db, do you think you could import it and use it as is in Starlight?

Yes! Didn't know it was available to external packages. After this PR it will be exactly the same as Starlight's so we could definitely reuse it.

const errorList = err.issues.map(
(issue) => ` ! ${bold(issue.path.join('.'))} ${red(issue.message + '.')}`,
const errorList = err.issues.map((issue) =>
`! ${renderErrorMarkdown(issue.message, 'cli')}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My highly-featured markdown renderer keep seeing usage 🫡

@delucis delucis merged commit 03958d9 into main Dec 5, 2024
16 checks passed
@delucis delucis deleted the chris/better-error-messages branch December 5, 2024 12:48
@astrobot-houston astrobot-houston mentioned this pull request Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants