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

failOnUnknown() should allow setting of error type #1245

Open
nnchang opened this issue Feb 18, 2021 · 2 comments
Open

failOnUnknown() should allow setting of error type #1245

nnchang opened this issue Feb 18, 2021 · 2 comments

Comments

@nnchang
Copy link

nnchang commented Feb 18, 2021

What happened?

As a java service, communicating with a storage backend via conjure, I used .failOnUnknown() when writing a visitor for the response object. I dug in though and noticed this would throw an illegal-argument exception and trigger an HTTP 400 response.

What did you want to happen?

Since this would be my application complaining about an unknown type coming from an underlying service, this definitely seems like an internal server error, HTTP 500. I would like to be able to pass an ErrorType into the .failOnUnknown() method, so I can control what HTTP status it would return without an awkward try-catch structure.

@carterkozak
Copy link
Contributor

If you send a request object which is missing fields (or contains additional fields the server doesn't understand) you'll receive a 400 response. I would consider this case equivalent: The client sends data that the server doesn't understand/expect, meaning the client has made a bad request.
You could provide an onUnknown function instead of using the default .failOnUnknown() if there's a niche use case for presenting a 5xx, but I would recommend against doing that by default.

@nnchang
Copy link
Author

nnchang commented Feb 18, 2021

I'm confused; in this case, my service is the client, and is operating on the response from an upstream service. In this case, someone sent me a request, I just tried to store it, and the unknown I'm handling is when I'm looking at the response from the storage layer. If I don't know how to interpret the response from the storage layer, that's not a bad request on my client's part, it's a bug in my code or a version mismatch between my service and the storage layer.

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

2 participants