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

Improving SDK error/response handling #19539

Merged
merged 16 commits into from
Sep 4, 2023
Merged

Improving SDK error/response handling #19539

merged 16 commits into from
Sep 4, 2023

Conversation

br41nslug
Copy link
Member

@br41nslug br41nslug commented Aug 28, 2023

Fixes #19304

Added the response object in the thrown error for any request if the error is one returned from Directus.

const result = await client.request(readItems('non-existent'))
// throws
{
  errors: [ [Object] ], // can potentially be a string for non-directus errors
  response: Response { ... }
}

Added the original request object to the onResponse(result, request) handler

@changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2023

🦋 Changeset detected

Latest commit: 4564527

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

This PR includes changesets to release 1 package
Name Type
@directus/sdk Minor

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

@br41nslug br41nslug requested review from a team, paescuj, rijkvanzanten and jaads and removed request for a team August 28, 2023 12:20
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

Very nice 🚀

@rijkvanzanten
Copy link
Member

@br41nslug Should we consider this a breaking change?

@br41nslug
Copy link
Member Author

Hmm, the error output could definitely be a breaking change for those relying on the error structure as it was 🤔

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

Tested the latest state ✅

Copy link
Member

@jaads jaads left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@br41nslug br41nslug merged commit e34e49f into main Sep 4, 2023
6 checks passed
@br41nslug br41nslug deleted the sdk-better-events branch September 4, 2023 11:58
@github-actions github-actions bot added this to the Next Release milestone Sep 4, 2023
br-rafaelbarros pushed a commit to personal-forks/directus-source that referenced this pull request Nov 7, 2023
* improving error/response handling

* updated request function to expose the response on error

* updated the composables where needed

* Create few-rules-talk.md

* ran prettier

* undid unintended type change

* added missing awaits for onResponse

* Update few-rules-talk.md

* Mark it as major change

* unpack directus errors instead of nesting

* ran prettier

* Update .changeset/few-rules-talk.md

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>

---------

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SDK should report response.status on fetch failure
4 participants