-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Feature Request: Create new ts-rest client that throws exceptions for error responses #520
Comments
Thank you for your proposal! Pretty well thought out, but you've overlooked a major point, which is why @ts-rest/react-query exists in the first place, and which solves this entire use-case, which I'll elaborate on later. The thing about ts-rest is that we try to make its usage as convenient as possible while being as unopinionated as much as possible. I believe we have struck this right balance. Our core client uses A such, we have provided functionality in order to allow everyone to customize the "client" to behave however they want it, or even swap out In your case you can just simply wrap the tsRestFetcher and do the error handling in a custom fetcher. const client = initQueryClient(postsApi, {
baseUrl: 'http://localhost:5003',
baseHeaders: {},
api: async (args) => {
const response = await tsRestFetchApi(args);
if (!String(result.status).startsWith('2')) {
throw response;
// or wrap with your own error, whatever
}
return response;
},
}); Now for react query, const { mutate } = useMutation<
ClientInferResponses<typeof contract.addPost, SuccessfulHttpStatusCode, 'force'>,
ClientInferResponses<typeof contract.addPost, ErrorHttpStatusCode, 'ignore'> // or whatever you wrap the error with
>({
mutationFn: (newPost) => client.addPost(newPost),
onSuccess: (response) => {
},
onError: (error) => {
// or type guard here instead of using the type generics
},
}); For the error typing you will still have to either manually type Anyway it seems that your problem with using However, the types and functions to assist with building this functionality yourself is already there. |
@Gabrola Regarding my usage of While Putting aside my personal preferences and specific use case, I wonder if there's a way to engage the community and discuss the desired behavior of the core client. One observation that leads me to believe that throwing errors might be more desirable is the implementation in I’d appreciate any insights or thoughts on PR #522 whenever you have a moment. As a temporary solution, I've created a utility function in my code that can be chained to the response promise. This utility returns the success response but throws an error response when necessary: export function successBody<TStatus extends SuccessfulHttpStatusCode, TRes extends Res>(
response: TRes,
status?: TStatus,
): (TRes & { status: TStatus extends undefined ? SuccessfulHttpStatusCode : TStatus })['body'] {
if (status != null ? response.status === status : response.status >= 200 && response.status < 300)
return response.body;
throw new TsRestError(response);
}
const { status, body: posts } = client.getPosts({...}).then(successBody);
// status is 2xx, posts is of type Post[]
// Wrap in a try-catch block to handle errors with status codes outside the 2xx range However, I'm not particularly fond of this approach, as I need to remember to include it in every method call. |
@toteto I left you some comments on the PR. However, I still don't believe much in the utility of throwing on Even if you were to manually use tanstack query, all of the new types in the regular client are still useless because you still have to manually type the error object, and since the error type is the second generic on Regarding not using I cannot justify including code for a feature, just to cover a very specific use-case that has not been requested by anyone else yet. |
And yes, regarding the incorrectly typed |
@Gabrola is it possible to use a client created with const client = initQueryClient(...)
client.getPost.useQuery() // -- does work
// then, at some place in code, I need raw `client.getPost()`
await client.getPost() // -- is not a function, any alternative syntax? Should I create 2 clients: one for higher- and another for lower-level API handling? I use React-Query but it's often necessary to make direct calls, without |
@toteto yeah, familiar situation 😅 . IMO it's easier to remap your understanding of "errors" than to fight with tooling.
From the client point of view 404, 500, etc. response status codes are not errors but expected scenarios. So you handle them in the same branch as 200. If you think about it like this, then everything behaves as expected. |
@ivan-kleshnin there are |
Hi @Gabrola Is there an ETA on v4? Or is there some way we can try and contribute to it's development? |
We are currently utilizing the
ts-rest
library on both the client and server side of our application. The client side is built with React and usesreact-query
. However, we are not using the@ts-rest/react-query
client due to compatibility issues with our usage patterns.Our previous experience with
Zodios
andAxios
for executing requests has led us to observe that the resolution of all responses (including non-success ones) can result in weird code flow. This issue is also observable in the provided examples.Example
The above example presents several issues:
onSuccess
callback. This includes error paths, which is not ideal. Error-handling should not be performed in themutationFn
, as it results in a non-idealresponse
type (if we returnundefined
in themutationFn
, the type ofresponse
will beResponse | undefined
).onError
callback handles errors for the second time.isError
/isSuccess
flags are incorrect, as an error response can be considered a success.Proposal
To address these issues, we propose the following:
isTsRestError(route: AppRoute | AppRouter, status : ErrorHttpStatus, error: Error) : error is ...
{ status: 200, body: ...}, { status: 201, isError: true, body: ... }
). This could be a router configuration or client configuration similar to theaxios.validateStatus
function: axios-http.com/docs/handling_errorsExample from above with
isError
/isSuccess
flags are correctThe text was updated successfully, but these errors were encountered: