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

Feature: option to disable default errorType() #118

Closed
gbcreation opened this issue Dec 10, 2021 · 7 comments
Closed

Feature: option to disable default errorType() #118

gbcreation opened this issue Dec 10, 2021 · 7 comments
Labels

Comments

@gbcreation
Copy link

First of all, thank you for making Wretch. That's a very useful Fetch wrapper.

When an error Response is returned by fetch(), Wretch always consumes it through a call to text() or json() depending on the configuration of errorType(). However, it can be useful some times to get the unconsumed raw response and let the error catcher handle it.

Currently, it is not possible to disable the consuming of the reponse. Maybe a call to errorType() without parameter could be used for that. WDYT?

@elbywan
Copy link
Owner

elbywan commented Dec 11, 2021

Hey @gbcreation,

First of all, thank you for making Wretch. That's a very useful Fetch wrapper.

Thanks! 🙇

However, it can be useful some times to get the unconsumed raw response and let the error catcher handle it.
Maybe a call to errorType() without parameter could be used for that. WDYT?

I think this is a very uncommon usecase, and using errorType() without parameter would be breaking since right now it defaults to calling .text() on the Response. So I'm not keen on doing that…

Currently, it is not possible to disable the consuming of the reponse.

…but the good thing is that you can achieve this already using a middleware 😄.

// Define a middleware that will intercept the response.
const errorMiddleware = next => (url, opts) => {
  return next(url, opts).then(response => {
    // Define a "_msg" function that will resolve to an empty string.
    // If you want a custom error message you can put anything inside…
    response._msg = () => Promise.resolve("")
    return response
  })
}

// Configure the middleware and the errorType with the "_msg" function here…
const w = wretch("https://httpstat.us").errorType("_msg").middlewares([errorMiddleware])

w.url("/400").get().res().catch(error =>
  // Access the response using the `.response` property.
  // it will not be consumed since "_msg" did not touch the body.
  // Here we consume it manually by calling ".text()"
  error.response.text()
).then(txt => console.log(txt))

@gbcreation
Copy link
Author

Hi @elbywan

Thank you for your suggestion. It works perfectly, but looks a little bit hacky IMO because we have to circumvent the fact that Wretch always processes the error Response. Here, a processing is performed to explicitly tell Wretch to do nothing, while the same result could be achieved if Wretch would indeed do nothing :)

I agree calling errorType() without parameter is not the best option because of the default behavior that calls .text(), but maybe a new 'raw" or "unconsumed" or ... ("better name" ?) parameter that would make Wretch let the error Response unconsumed would be a better approach.

@gbcreation
Copy link
Author

gbcreation commented Dec 13, 2021

When calling .errorType("_msg"), TypeScript complains with Argument of type '"_msg"' is not assignable to parameter of type '"text" | "json"'. Do you know how to tell TypeScript that "_msg" is an acceptable argument?

@elbywan
Copy link
Owner

elbywan commented Dec 13, 2021

Do you know how to tell TypeScript that "_msg" is an acceptable argument?

The bindings are wrong here, I'll need to update them to accept any string as an argument. In the meantime "_msg" as any should work. Thanks for reporting 👍.

Here, a processing is performed to explicitly tell Wretch to do nothing, while the same result could be achieved if Wretch would indeed do nothing :)

Wretch needs to populate the error message actually.

I agree calling errorType() without parameter is not the best option because of the default behavior that calls .text(), but maybe a new 'raw" or "unconsumed" or ... ("better name" ?) parameter that would make Wretch let the error Response unconsumed would be a better approach.

Again I think that the use case is so uncommon that it is not really worth it IMO. And adding a "magic string" argument sounds even more hacky to me 😉 .

@gbcreation
Copy link
Author

The bindings are wrong here, I'll need to update them to accept any string as an argument. In the meantime "_msg" as any should work.

Thank you for this temporary solution.

And adding a "magic string" argument sounds even more hacky to me 😉 .

Well, maybe a new .processError(true | false) (default is true) method would then be even less hacky 😄

elbywan added a commit that referenced this issue Dec 13, 2021
@elbywan
Copy link
Owner

elbywan commented Dec 13, 2021

@gbcreation Typescript bindings have been updated and released with v1.7.7 📦 ✨.

Regarding the original feature request, since it is a very uncommon use case and quite easily done using a simple middleware I'm sorry but I'm not up for implementing that in the lib itself (& by extension increasing the bundle size).

@gbcreation
Copy link
Author

Thanks for the types update.

Closing this issue as your suggested solution based on a middleware works.

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

No branches or pull requests

2 participants