Preserve server error-data
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. 24
if (!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! :)
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 https://github.com/the-bugging/react-use-downloader/issues/28, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZKISSSAOGCCKGDLRF2VETZI7GOXAVCNFSM6AAAAABJZJE75KVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM3DSNJRGU2DCOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>
+1 for this improvement. Error body is crucial to have in some cases
@all-contributors, please add @MarcRoemmelt for idea and code @all-contributors please add @nik-webdevelop for idea
@all-contributors please add @nik-webdevelop for ideas
Can you guys test out version 1.2.10? Thanks again for the contribution
@olavoparno thanks for the fix! After the update, I still cannot get access to the whore error response or, at least, the error response body. I created a pr just to show the idea.
Sometimes we get custom errors from BE with a body that includes useful info
const {error} = useDownloader();
if (isCustomError(error)) {
console.log(error.someUsefulErrorProperty)
}