react-use-downloader icon indicating copy to clipboard operation
react-use-downloader copied to clipboard

Preserve server error-data

Open MarcRoemmelt opened this issue 1 year ago • 1 comments

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! :)

MarcRoemmelt avatar Jun 24 '24 07:06 MarcRoemmelt

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: @.***>

olavoparno avatar Jun 25 '24 20:06 olavoparno

+1 for this improvement. Error body is crucial to have in some cases

nik-webdevelop avatar Mar 07 '25 16:03 nik-webdevelop

@all-contributors, please add @MarcRoemmelt for idea and code @all-contributors please add @nik-webdevelop for idea

olavoparno avatar Mar 10 '25 14:03 olavoparno

@olavoparno

I've put up a pull request to add @MarcRoemmelt! :tada:

allcontributors[bot] avatar Mar 10 '25 14:03 allcontributors[bot]

@all-contributors please add @nik-webdevelop for ideas

olavoparno avatar Mar 10 '25 14:03 olavoparno

Can you guys test out version 1.2.10? Thanks again for the contribution

olavoparno avatar Mar 10 '25 15:03 olavoparno

@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)
}

nik-webdevelop avatar Mar 17 '25 17:03 nik-webdevelop