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

Make it possible to get request method from error object #265

Closed
1 task done
phorcys420 opened this issue Aug 16, 2023 · 11 comments
Closed
1 task done

Make it possible to get request method from error object #265

phorcys420 opened this issue Aug 16, 2023 · 11 comments

Comments

@phorcys420
Copy link

phorcys420 commented Aug 16, 2023

Describe the feature

try {
  const res = await ofetch("https://url/that/returns/an/error");
} catch(e) {
  // typeof(e.request) === "string" -> true
  // I want to access request.method from the catch block
}

Is there a workaround ?
Am I doing this the wrong way around ?


The issue here is that FetchRequest's type boils down to string | Request, and a string is being returned, which is rather unhelpful when you also need to access other properties from the request object.

declare class FetchError<T = any> extends Error {
    name: string;
    request?: FetchRequest;
    response?: FetchResponse<T>;
    data?: T;
    status?: number;
    statusText?: string;
    statusCode?: number;
    statusMessage?: string;
}

Additional information

  • Would you be willing to help implement this feature?
@picunada
Copy link
Contributor

picunada commented Aug 17, 2023

Hello,

as you use URL string to make request, context will contain request as string.
Workaround is to use Request object instead of URL string, I think this is how original fetch was envisioned to work.

$fetch(new Request('http://api.com/post', {
    method: 'post',
    body: JSON.stringify({})
}), {
    // those will be overridden by Request object, even if not specified in Request object
    method: 'post',
    body: {
        
     }
})

example codesandbox

@KaKi87
Copy link

KaKi87 commented Aug 18, 2023

I would like to suggest adding the options object to that error object just like in the interceptor's error object. Thanks

@picunada
Copy link
Contributor

@KaKi87
I don't understand why would you need to have Request and FetchOptions objects in FetchError object, if you control the request and options that you send to server.
Can you specify examples where this feature is needed?

@KaKi87
Copy link

KaKi87 commented Aug 18, 2023

Actually, I was only making this suggestion to solve OP's request.

Personally, I don't have such a use case.

But, since Axios provides this as well, I suppose it can be useful.

Thanks

@phorcys420
Copy link
Author

as you use URL string to make request, context will contain request as string. Workaround is to use Request object instead of URL string

oh yeah that totally makes sense now

I think this is how original fetch was envisioned to work.

they don't advertise it much to be honest


for the record, I need this feature for logging purposes, I am migrating some library from snekfetch to ofetch and when there's an error there's logging that contains the request method and I wanted to keep this behavior since I think it's valuable.

logging the endpoint and returned body might not be enough given that the same endpoint will not do the same thing when requested with GET, PUT or POST.

thank you for your answer, I am marking this as resolved.

@KaKi87
Copy link

KaKi87 commented Aug 18, 2023

I think this issue should remain open because the Request workaround removes all the automation that we use ofetch for. Thanks

@phorcys420 phorcys420 reopened this Aug 18, 2023
@phorcys420
Copy link
Author

phorcys420 commented Aug 18, 2023

I do agree.
I didn't look at the workaround much when I first saw it but think that it's totally redundant to do so and isn't as user-friendly as it could be.

As @KaKi87 said, it kind of eradicates the whole point of using ofetch in the first point.


I took a quick glance at the code and I think context.options should be passed to createFetchError.
https://github.com/unjs/ofetch/blob/19b79fa5cbf24cda0a81bf768c636e01eef82541/src/fetch.ts#L121C19-L121C35


Another (better) solution would be to simply use context.request instead of context.options whenever writing the new parameters.

what I mean is that whenever a string is passed as the first argument, it gets turned into a Request automatically and ofetch adds the parameters to that Request object instead of context.options, which is then passed to the error object like usual.


I am still willing to PR this if you feel like this is a valuable thing like I do, it's also pretty easy.

@phorcys420
Copy link
Author

I don't understand why would you need to have Request and FetchOptions objects in FetchError object, if you control the request and options that you send to server. Can you specify examples where this feature is needed?

simply put, the codebase is built in such a way that when the promise is resolved, there is no access to the request options.

@luizzappa
Copy link

looking for that too

@pi0
Copy link
Member

pi0 commented Aug 23, 2023

Should be supported via error.options after #270 being released 👍🏼

@pi0 pi0 closed this as completed Aug 23, 2023
@phorcys420
Copy link
Author

thanks!

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

5 participants