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

useFetch: [Feature request] Expose whole the http response on error (in onFetchError) #1144

Closed
red010182 opened this issue Jan 17, 2022 · 5 comments · Fixed by #1473
Closed

Comments

@red010182
Copy link

red010182 commented Jan 17, 2022

Please consider expose the whole http response in onFetchError.

e.g. ctx.error.response (like axios did)

Currently, there's nothing useful in ctx of onFetchError, I can't even get status code nor url.
It's really hard to customize a useful error handler middleware without key information.

const fetcher = createFetch({
  baseUrl: 'http://localhost:5678/',
  options: {
    async onFetchError(ctx) {
      // only ctx.data and ctx.error
      // can't get status code, request url or request parameters
      console.error(ctx) 
      return ctx
    },
  },
})

Feature request:

ctx.error.response = { ... } // http response object, contains status code, url, parameters...
@red010182 red010182 changed the title useFetch: Please expose whole the http response in onFetchError useFetch: [Feature request] Expose whole the http response on error (in onFetchError) Jan 18, 2022
@Delicious-Bacon
Copy link

I would like to point out one thing that I noticed.

Like you mentioned, right now, we only have ctx.data and ctx.error available in the onFetchError interceptor, which are not of much use apart from mutating them. I guess that this interceptor is only used for setting default fallbacks to the data.

But the afterFetch interceptor has both the response and data properties in the ctx (source code):

ctx: {
  response,
  data
}

While the onFetchError interceptor triggers only on errors, the afterFetch interceptor should trigger on any fetch response, even those with error codes, but the documentation only says The afterFetch option can intercept the response data before it is updated. (Documentation)

I would say we're supposed to utilize our handlers in afterFetch interceptor since it has the needed information, and it triggers on server responses (the onFetchError interceptor triggers whenever there is error, so it would trigger when the server is down, for example).

An example of afterFetch interceptor:

const fetcher = createFetch({
  // baseUrl: '...',
  options: {
    afterFetch({ response }) {
      const { status, url } = response;

      if (status === 200) {
        console.log('Successfully fetched data @:', url);
      } else if (status === 404) {
        console.log('404 not found @:', url);
      }
    },
  },
  fetchOptions: {
    // ...options
  },
});

@red010182
Copy link
Author

Thanks. This may be a workaround for now. It would be convenient to get response (especially status code and url) in onFetchError for me.

@wheatjs
Copy link
Member

wheatjs commented Feb 9, 2022

Going to reopen this as I would like to include it. I'll see if I can get around to it soon

@wheatjs wheatjs reopened this Feb 9, 2022
@Ragura
Copy link

Ragura commented Feb 21, 2022

@Delicious-Bacon, you should note that the documentation states that afterFetch() is only run on 2xx status codes, not after each fetch (currently has an open pull request to correctly implement this (#1266)). This makes it even more important to have access to the extra data such as the status code in the ctx argument of onFetchError(), because errors shouldn't even appear in the afterFetch() hook.

@Glandos
Copy link
Contributor

Glandos commented Mar 29, 2022

It seems that the design of afterFetch and onFetchError is confusing. Issues raised in #553 and #526 are similar to this one, and were "fixed" by #572 that move the error after the call to afterFetch, leading to calling it on every requests, even failed ones.

Then, #1266 was opened because the "doc" was not matching the code, but the PR #1271 linked to this kind of revert #572, since it checks the status code and doesn't call afterFetch. My best guess was that the code was right and the "doc" should have been updated in #572.

And now, we all have the same issue: we need to have the full response to do some useful interceptions. So we can either:

  • Call afterFetch everytime. After all, the HTML documentation doesn't talk about errors in afterFetch workflow. It was only the JSDoc that mentionned it.
  • Enhance onFetchError and give the response.

I don't have strong opinions on this. But we need one of those.

robin-dongbin added a commit to robin-dongbin/vueuse that referenced this issue Mar 31, 2022
antfu pushed a commit that referenced this issue Apr 3, 2022
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

Successfully merging a pull request may close this issue.

5 participants