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

RESTDataSource, cannot easily throw error if response is 200 #32

Closed
jounii opened this issue Feb 24, 2020 · 1 comment · Fixed by #115
Closed

RESTDataSource, cannot easily throw error if response is 200 #32

jounii opened this issue Feb 24, 2020 · 1 comment · Fixed by #115

Comments

@jounii
Copy link

jounii commented Feb 24, 2020

Right now the didReceiveResponse and errorFromResponse pair works fairly well with non-2xx statuses. Unfortunately there exists REST services (legacy) which return 200 OK with error object instead as body. If trying to check this in didReceiveResponse and then throwing error through errorFromResponse causes problem with the response.json() parsing, as you cannot call that one twice.

Either the parseBody should know better (processing time memoize) or errorFromResponse should accept body passed and use that instead of calling parseBody. This would allow passing already parsed body to be passed to super.didReceiveResponse.

Right now my solution is just to copy-paste (partially) code for of ApolloError object create from errorFromResponse to the custom didReceiveResponse.

@Webbist-dev
Copy link

Can I see your solution for this, I am having a similar dilemma

@glasser glasser transferred this issue from apollographql/apollo-server Oct 11, 2022
glasser added a commit that referenced this issue Dec 8, 2022
We previously removed `didReceiveResponse` which let you override the
main "is it an error or not" logic. This restores it in an
error-specific way. This means you can throw errors even on 200s if you
want (or vice versa).

This also changes the `errorFromResponse` hook to take options instead
of a single argument, in case you want the error to reflect more data.

One negative change is you can no longer prevent `parseBody` from being
called, though you can always override `parseBody` to do less.

Fixes #32.
glasser added a commit that referenced this issue Dec 9, 2022
We previously removed `didReceiveResponse` which let you override the
main "is it an error or not" logic. This restores it in an
error-specific way. This means you can throw errors even on 200s if you
want (or vice versa).

This also changes the `errorFromResponse` hook to take options instead
of a single argument, in case you want the error to reflect more data.

One negative change is you can no longer prevent `parseBody` from being
called, though you can always override `parseBody` to do less.

Fixes #32.
glasser added a commit that referenced this issue Dec 9, 2022
We previously removed `didReceiveResponse` which let you override the
main "is it an error or not" logic. This restores it in an
error-specific way. This means you can throw errors even on 200s if you
want (or vice versa).

This also changes the `errorFromResponse` hook to take options instead
of a single argument, in case you want the error to reflect more data.

One negative change is you can no longer prevent `parseBody` from being
called, though you can always override `parseBody` to do less.

Fixes #32.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants