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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix message for unknown API errors #22379

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

paescuj
Copy link
Member

@paescuj paescuj commented May 3, 2024

Scope

  • Fix broken logic to get error message of unknown errors (e.g. from a Flow) for admin users
    • Brackets were missing, which resulted in the error object itself being used as the message
    • Example Flow
      Before
      After
  • Clean-up of error handler
    • Wrap in async handler, similar to what we're using in other middleware/handlers - makes it easier to read and allows us to easily catch possible errors in the handler itself (unexpected, but would leak filesystem structure info if uncaught)
    • Add comments to reveal some of the magic happening here 馃獎
    • Reusable fallback error
    • Prevent adding undefined extensions data / stack traces to error data (in dev mode)
    • Don't use toArray util to transform errors - would split string errors by commas
    • Better typing
  • Shared InternalServerError

Potential Risks / Drawbacks

None

Review Notes / Questions

None


Fixes #22420, partially addresses #22416

Copy link

changeset-bot bot commented May 3, 2024

馃 Changeset detected

Latest commit: 0e5a045

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

This PR includes changesets to release 6 packages
Name Type
@directus/api Patch
@directus/errors Minor
directus Patch
@directus/extensions-registry Patch
@directus/memory Patch
@directus/validation Patch

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

Copy link
Contributor

@hanneskuettner hanneskuettner left a comment

Choose a reason for hiding this comment

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

One minor nitpick, otherwise it looks super clean

api/src/middleware/error-handler.ts Outdated Show resolved Hide resolved
Co-authored-by: Hannes K眉ttner <4376726+hanneskuettner@users.noreply.github.com>
@paescuj paescuj force-pushed the enhance-api-error-handler branch from ffb7eea to 21c19cd Compare May 3, 2024 10:55
@alexchopin alexchopin added this to the Next Minor Release milestone May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 馃憖 In review
Development

Successfully merging this pull request may close these issues.

Error message missing in the popup dialog
4 participants