hey-api / openapi-ts

✨ Turn your OpenAPI specification into a beautiful TypeScript client
https://heyapi.vercel.app
Other
1.02k stars 86 forks source link

Tanstack Query Incorrect onError Types #1029

Open Daynil opened 1 week ago

Daynil commented 1 week ago

Description

I've run into an issue with the types of the error returned in a mutation. My mutation correctly has my error type returned based on my OpenAPI spec in the onError callback. However, when actually logging the error out, I get the model of the error wrapped in an Error.message.

//...generated:
export type AuthLoginError = {
    title?: string;
    detail?: string;
};

const loginUser = useMutation({
  ...authLoginMutation(),
  // On hover, this error correctly shows `AuthLoginError`
  onError: (error) => {
    // However, here error.message is the raw string: {"title":"Login error","detail":"Please check your information and try again."}
    console.log(error.message);
}

Looking at the network tab, the response was correctly sent as a 401 with payload {"title":"Login error","detail":"Please check your information and try again."}.

It seems this occurs because, with the throwOnError option set to true, we get our payload wrapped in an error:

if(r.throwOnError) throw new Error(f);

The throwOnError option makes sense - this is what tanstack-query expects when we get a 300+ status code. However, how do we then leverage the types provided by the OpenAPI spec if everything is wrapped as a string in Error.message?

Apologies I couldn't provide a repro using the stackblitz provided since the API spec used in the example doesn't provide a model for their error codes.

Reproducible example or configuration

No response

OpenAPI specification (optional)

No response

System information (optional)

No response

mrlubos commented 1 week ago

@Daynil is this the Fetch client? I think it should be changed to re-throw just like the Axios one

Daynil commented 1 week ago

@mrlubos Thank you for having a look! Yes it is the fetch client. I tried switching to axios and it does infer types properly there, so I'll stick with that for now.

Daynil commented 6 days ago

@mrlubos

I believe I found another bug in the axios implementation of error returns. The axios mutation errors are correctly typed, but the query errors are incorrectly typed as generic untyped Errors, when in fact they should also be AxiosErrors (same as the mutations).

// Currently generated mutation options (types on postSomethingMutation.error are correct)
export const postSomethingMutation = () => {
    const mutationOptions: UseMutationOptions<
        postSomethingResponse,
        AxiosError<PostSomethingError>,
        Options<PostSomethingData>
    > = {
        mutationFn: async (options) => {
            const { data } = await postSomething({
                ...options,
                throwOnError: true
            });
            return data;
        }
    };
    return mutationOptions;
};

// Currently generated query options (getSomethingQuery.error is not correctly typed)
export const getSomethingOptions = (
    options: Options<GetSomethingData>
) => {
    return queryOptions({
        queryFn: async ({ queryKey }) => {
            const { data } = await getSomething({
                ...options,
                ...queryKey[0],
                throwOnError: true
            });
            return data;
        },
        queryKey: getSomethingQueryKey(options)
    });
};

// Proposed fix for generated query options (getSomethingQuery.error correctly typed
export const getSomethingOptions = (options: Options<GetSomethingData>) => {
    const queryOptions: UseQueryOptions<
        GetSomethingResponse,
        AxiosError<GetSomethingError>
    > = {
        queryFn: async ({ queryKey }) => {
            const { data } = await getSomething({
                ...options,
                ...queryKey[0],
                throwOnError: true
            });
            return data;
        },
        queryKey: getSomethingQueryKey(options)
    };
    return queryOptions;
};
mrlubos commented 6 days ago

Thanks @Daynil, yes I think queries need better type support, that would also resolve the issue with Vue refs