-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Preserve server error-data #28
Comments
Hey there, thanks for the issue. I'd appreciate it if you opened a PR,
truly.
Thank you again.
…On Mon, Jun 24, 2024 at 4:57 AM Marc Römmelt ***@***.***> wrote:
Hi! Thanks for the useful lib. We have been using it for a while and have
a small suggestion for an improvement.
*Is your feature request related to a problem? Please describe.*
The current implementation ignores the error returned by the server, which
may contain useful information (e.g. please complete step xyc before
downloading this file). Instead it throws a generic new Error(${response.status}
${response.type} ${response.statusText}) for all error status codes.
*Describe the solution you'd like*
It would be useful to capture the error contained in the request body and
use that as the error (if there is one).
*Describe alternatives you've considered*
Verifying that the download is generally possible/allowed could be
implemented via secondary routes instead of relying on the error for
feedback. However, that seems like an unnecessary workaround. Errors should
be usable in the client.
*Additional context*
// instead of throwing this error here, throw an error based on the request body// l. 24if (!response.ok) {
throw Error(`${response.status} ${response.type} ${response.statusText}`);}
// e.g.// l. 231.then(resolverWithProgress).then((response) => {
if (response.ok) return response;
return response.json().then((errorObj) => /* set or throw custom error and handle in error-handler below */);}).then((data) => {
return data.blob();})
I'm happy to open a PR for this feature if you like! :)
—
Reply to this email directly, view it on GitHub
<#28>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABZKISSSAOGCCKGDLRF2VETZI7GOXAVCNFSM6AAAAABJZJE75KVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM3DSNJRGU2DCOI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi! Thanks for the useful lib. We have been using it for a while and have a small suggestion for an improvement.
Is your feature request related to a problem? Please describe.
The current implementation ignores the error returned by the server, which may contain useful information (e.g. please complete step xyc before downloading this file). Instead it throws a generic
new Error(
${response.status} ${response.type} ${response.statusText})
for all error status codes.Describe the solution you'd like
It would be useful to capture the error contained in the request body and use that as the error (if there is one).
Describe alternatives you've considered
Verifying that the download is generally possible/allowed could be implemented via secondary routes instead of relying on the error for feedback. However, that seems like an unnecessary workaround. Errors should be usable in the client.
Additional context
I'm happy to open a PR for this feature if you like! :)
The text was updated successfully, but these errors were encountered: