onOkResponse interceptor

[Amar-Gill]

Describe the feature

Would anyone think it's helpful to add a new ofetch interceptor? https://github.com/unjs/ofetch#%EF%B8%8F-interceptors

I'm thinking about an onOkResponse interceptor.

Because in my project I have a custom useFetch call that has an onResponseError interceptor as well as an onResponse handler (for successful responses).

Reason is, I must wrap my logic in onResponse handler with if (ctx.response.ok) { .... In the quest to eliminate boilerplate, I wonder if it would be valuable to add the onOkResponse interceptor, that only runs if ctx.response.ok is true - so the exact opposite of onResponseError which only runs if ctx.response.error is not true.

This idea stemmed from me debugging why my interceptor wouldn't run for a couple hours. The root cause was a silent error in onResponse which blocked onResponseError from running. (Maybe this should be a separate issue).

My onResponse handler depends on the request body to be defined, but during an error response it isn't defined and I got a silent error. Only after wrapping my logic with if (ctx.response.ok) does my onResponseError interceptor run.

Here is a minimal reproduction of the interceptors getting blocked, I think it will work for any other set of interceptors:

const { data } = await useFetch('/api/has-error', {
  onResponse() {
    console.log('On response handler');
    throw new Error('todo');
  },
  onResponseError() {
    // this does not run because error thrown from `onResponse()` interceptor
    console.log('On response error handler');
  },
});

Since an error is thrown from onResponse -- onResponseError will not run.

Additional information

• [X] Would you be willing to help implement this feature?

[sevillaarvin]

Thanks for the info. I thought that's how it was supposed to work:

• onResponse handles response.ok, while
• onResponseError handles !response.ok

When I tested in my code, with !response.ok, onResponse gets called first, then onResponseError.

[Amar-Gill]

onResponse will always run, whether the response is an error response or not. I was thinking of adding a separate interceptor for just the response.ok case.

But TBH I think I might be using an anti-pattern in my code:

    onResponse(ctx) {
      if (ctx.response.ok) {
        const projIdOfReport = ctx.response._data.report.projectId;

        activeProject.value = projects.value.find(
          (proj) => proj.id === projIdOfReport
        );
      }
    },

I am accessing the response._data property. My issue was around having to wrap the code in the if (response.ok) -- but I shouldn't be using _data in my code because that is meant for Nuxt internals AFAIK.

[sevillaarvin]

I am accessing the response._data property. My issue was around having to wrap the code in the if (response.ok) -- but I shouldn't be using _data in my code because that is meant for Nuxt internals AFAIK.

I wouldn't worry about response._data. I got curious and looked into it, and from what I gathered it's just a workaround for node-fetch, but _data is still meant to be used publicly.

• pull request - https://github.com/unjs/ofetch/pull/49/files
• issue fixed - https://github.com/nuxt/nuxt/issues/13255
• root cause - https://github.com/unjs/ofetch/issues/48
• rationale - https://github.com/node-fetch/node-fetch/issues/1000

Though I agree that response.data would look more pleasing than response._data.

[Amar-Gill]

Interesting! Thanks for sharing that context.

So in this case, I do think the proposed enhancement here could be useful. Because to access response._data you must wrap it in if (response.ok) { ... otherwise the interceptor will silently fail when the response is an error.

And a consequence of the silent failure in onResponse is the intended error handler onResponseError does not execute.

Checking that response.ok is truthy is not a big deal for me personally, but I think it would be nice to eliminate boilerplate if we can.

[Smef]

I think it would be very helpful to handle these separately. A pattern like onSuccess, onError, and onFinish makes a lot of sense to me. onResponse would fire before either response hander (if it stuck around), and onFinish would fire after the other handlers.