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

Create hook that controls rejection process for m.request #2000

Closed
JAForbes opened this issue Oct 24, 2017 · 2 comments
Closed

Create hook that controls rejection process for m.request #2000

JAForbes opened this issue Oct 24, 2017 · 2 comments

Comments

@JAForbes
Copy link
Collaborator

I personally think mithril shouldn't automatically turn responses into rejections based on status code. mithril cannot know the response structure or semantics of a particular api ahead of time, and wrapping the response in an error can lead to bugs and indirection.

Possible Solution

I think if the existing behaviour was moved to a hook that could be overridden, then we could disable this feature via hook=identity

e.g.

m.request({ ... onHTTPError: i => i })

Passing values to the error constructor is not isomorphic, I recently discovered a message property was being absorbed into the Error prototype in firefox when mithril was converting the response to an Error. In practice that meant a simple stringify of the response would be missing an important property.

It's possible to work around this, but for many contexts its unwanted indirection. Moving this logic to a hook is a relatively simple non breaking change.

@JAForbes
Copy link
Collaborator Author

After speaking with @spacejack in the gitter, I think a better solution would be to change the behaviour of extract.

If you opt into extract, mithril should no longer perform any logic or transformations after that point. If you want to manually transform the response, its logical that you will be deciding how to decode responses and transform them to fit your error handling strategy.

Link

@spacejack: haha... oh man I just realized why I didn't get burned by that, it's because I was throwing when I encountered a 4xx. So I guess my exception was thrown first
But yeah I assumed it didn't throw

@JAForbes
well yeah, I'm saying that should be what extract is: explicit handling of the response that you opt in to, including the entire status code range

@spacejack

Yes I agree, I think it should be what I thought it was

@tivac
Copy link
Contributor

tivac commented Dec 21, 2017

Fixed in 80b6a1a

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

No branches or pull requests

2 participants