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

feat: expose ClientError statuses derived from API #515

Closed
wants to merge 1 commit into from

Conversation

TylorS
Copy link

@TylorS TylorS commented Apr 2, 2024

Extracts all of the non-200 statuses from an Endpoint to be utilized in the returned ClientError when possible.

@sukovanej
Copy link
Owner

Hey, I'm afraid such a contract wouldn't be generally valid because on the server side you can currently fail with whatever ServerError that doesn't necessarily need to be communicated in the Api. Also, if you're modeling an external service which you have no control over can get in trouble if the server is behind a reverse proxy (typical example is a standard kubernetes setup) and the upstream becomes unavailable, then the proxy would return 503 which is usually not documented and you'd have a value in the error channel not captured in the type.

@sukovanej
Copy link
Owner

One option to enable this strict error mode would be to perform the same response validation on the error channel we perform for the success and handle specified non-2xx responses using tracked errors and die for the rest.

@TylorS
Copy link
Author

TylorS commented Apr 4, 2024

Yeah, I relied previously on being able to match over all of the status codes from the responses returned from the client.

This was more of a stopgap to gain back that functionality, such that I can create functions to convert from these non-2xx errors to domain-specific errors within Effect with type-safety. I don't prefer the approach in this PR, because the payloads are again untyped within a ClientError.

Instead of just returning "success" status codes, returning "expected" status codes with their payloads would definitely be ideal to me

@TylorS
Copy link
Author

TylorS commented Apr 18, 2024

Closing this in favor of #533, to find a solution similar to that you described above.

I attempted a summary, but if I got anything off feel free to edit it as well or let me know and I can edit it as we see fit

@TylorS TylorS closed this Apr 18, 2024
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 this pull request may close these issues.

2 participants