asynchronous-injection
asynchronous-injection copied to clipboard
ploeh
Returning InternalServerError from Controller
Just saw your NDC video. The presentation was fabulous but one thing struck me from the beginning - returning 500 if table is unavailable. For a client it looks like something exceptional happened on the server but isn't it a common thing that a table is full?
It's a minor thing but I'm wondering what if I'd like to return Reservation on both cases instead of an integer? Because now a client has to deal with this 'hot potato'.
If not 500 Internal Server Error, then which HTTP status code would you suggest instead?
like to return Reservation on both cases
But in the second case, there's really no reservation...
If not
500 Internal Server Error, then which HTTP status code would you suggest instead?
200 OK, server correctly processed the request but the reservation cannot be made and you get the reservation with isAccepted = false.
like to return Reservation on both cases
But in the second case, there's really no reservation...
There's still a reservation but not approved.
What, would you say, is the intent of making an HTTP Post request against the /reservations resource?
Maybe it's just me but I would understood 500 more like "restaurant is on fire" if I wouldn't read the response I could even just log an error and try to send the request again after some period.
The status of standard HTTP error codes makes this less than ideal, but essentially, the 200 range means that all is fine, the 300 range is for redirects, the 400 range is for client errors, and the 500 range is for server-side errors.
The problem is that according to RFC 7231 there isn't that many error codes to choose from in the 500 range. Neither 501 Not Implemented, 502 Bad Gateway, 503 Service Unavailable, 504 Gateway Timeout, nor 505 HTTP Version Not Supported fit the scenario.
You could argue that neither does 500 Internal Server Error:
The 500 (Internal Server Error) status code indicates that the server encountered an unexpected condition that prevented it from fulfilling the request.
Among all the 500 error codes, this one is the closest to being appropriate. It's basically just the word unexpected that causes trouble. What if we removed it?
The 500 (Internal Server Error) status code indicates that the server encountered [a ...] condition that prevented it from fulfilling the request.
This seems to fit the scenario where the system has to reject the reservation request.
You can't lightly choose to reinterpret the HTTP standards to fit your own purposes, but sometimes you're stuck between a rock and a hard place. Is there a better option?
200 OK
Is this appropriate? Again, I don't think we can proceed until we've agreed on the question I already put forth: What is the intent of making an HTTP Post request against the /reservations resource?
I would have agree if the unexpected wasn't there, which for me is an essence :)
But let's proceed. What if there was a real exception during request execution. What would you return? Probably 500. Then client would have to parse the body to understand it's not the reservation that was rejected but an exception "sorry, try again later", The table could still be available.
The same way it goes with 200 but this time, a client knows that everything was correctly processed by the server and the reservation was approved/rejected. Again, to know that a client has to parse the body.
I think the HTTP status code alone is not enough to convey the response and here we disagree I guess.
I should probably make some of my design choices more explicit. I don't think I've ever stated this, but the choice of using 500 Internal Server Error is based on the overall design principle of level 3 REST.
In RESTful API design, you explicitly leverage the HTTP protocol to transfer state representations. That includes not only verbs, but also status codes.
The best resource for learning RESTful design is, in my opinion, the RESTful Web Services Cookbook, which has this to say about returning error codes in 200 responses:
"One common mistake that some web services make is to return a status code that reflects success (status codes from
200to206and from300to307) but include a message body that describes the error condition."
"Doing this prevents HTTP-aware software from detecting errors. For example, a cache will store it as successful response and serve it to subsequent clients even when clients may be able to make a successful request."
A 200 OK response would indicate success, i.e. that the reservation was accepted, even if it wasn't. If a client developer isn't explicitly aware that the client software must check a non-standard status code, he or she may forget to do so. Why put such a burden on all client developers when a more standardised alternative exists?
A
200 OKresponse would indicate success, i.e. that the reservation was accepted, even if it wasn't. If a client developer isn't explicitly aware that the client software must check a non-standard status code, he or she may forget to do so.
I wrote about it regarding 500 code, client has to know anyway if this is a rejection or e.g an infrastructure issue. I could use the quote from the book, that HTTP-aware software could just log an error and activate a retry policy.
I've read the book (agree it's a good resource) however I think we understand what is an error differently. I split them into domain and infrastructure/internal errors. For me a domain error i.e. rejecting a reservation is not an error per se. Imagine an API for loan inquiry, again 500 would mean for me an infrastructure issue (e.g. banking system maintenance) not a rejection.
Thank you for a good discussion so far :)
HTTP-aware software could just log an error and activate a retry policy
The HTTP-aware software that the book refers to is routers and proxies out there on the internet. These devices have no knowledge of your specific API, but treat it as instructed by HTTP headers. They're not going to log anything, and you can't change how they behave.
For me a domain error i.e. rejecting a reservation is not an error per se
Why not?
Why not?
Firstly, they're expected upfront. Secondly, just because they might be negative in some context (reservation, loan) doesn't make them erroneous. So to answer your previous question - the intent of sending POST to /reservations endpoint is not to have it accepted but rather to make sure the restaurant won't be overbooked.
they're expected upfront
If that precludes a outcome from being an error, what would constitute an error according to your heuristics?
the intent of sending
POSTto/reservationsendpoint is not to have it accepted but rather to make sure the restaurant won't be overbooked.
So, as a user, I fill out a form on the internet with the intent to make sure that the restaurant isn't overbooked?
If that precludes a outcome from being an error, what would constitute an error according to your heuristics?
Actually, you're right. This is not a domain error. Error would be if user would provide past date or no date (then it'd be 400 I guess). It's just one of a possible responses.
So, as a user, I fill out a form on the internet with the intent to make sure that the restaurant isn't overbooked?
User's intent is probably to have a table that will be ready for him, not for him plus a few other people. If it's booked already, then he'll try to change the date or look for another restaurant but there's nothing wrong with it.
User's intent is probably to have a table that will be ready for him
I agree.
If no table is available as requested, would you say that the desired outcome was achieved?
We did a full circle probably :)
When sending a request there's no such thing as a single desired outcome. I'm asking for a table if available. No one has guaranteed there will be a table waiting for me, thus I'm not perceiving a rejection as an error. It's still a valid and desired outcome for me - "I have to look elsewhere". This is the functionality I expect from a reservation system.
Added some examples before, but the principle is pretty standard, I could quote the RFC:
- infrastructure errors/exceptions -
5xx - client errors -
4xx
In my opinion, the case is none of the above.
Probably we won't convince each other, perhaps a new status code 407 No Table should be invented.
Could you give an example of something that you think should result in an error?
@ploeh - I've just finished reading the Martin Fowler's article you linked. He described similar situation and suggested HTTP 409 code (conflict) in such case.
Usage of code 500 here introduces, in my opinion, ambiguity and confuses caller, because he doesn't know whether his reservation was rejected due to fire in the restaurant, or due to lack of the table. He will eventually use a telephone to call the restaurant and solve the case.
Could you give an example of something that you think should result in an error?
As I wrote, I don't think I would return an error 5xx other than infrastructure/exception.
He described similar situation and suggested HTTP 409 code (conflict) in such case.
As for 409 it's an interesting alternative to 200. The issue with 409 is that it indicates a client is able to solve the request and resubmit it. One could argue that it can be done by changing the date/time of the reservation :) (EDIT: Other one could argue that reservation with a different date/time is not the same reservation anymore) I could consider it as a compromise.
@PawelSzczygielski, I would tend to agree with @Crokus that the stipulation that the client should be able to resolve the issue and resubmit the request makes 409 Conflict suspect as well.
In general, the HTTP 1.1 specification has a strong subtext of describing the interaction between web servers and browsers. Even though Fielding is one of its authors, and also the 'inventor' of REST, I don't believe one can do RESTful API design while following the HTTP 1.1 spec to the letter.
You have to bend the rules a bit, one way or the other. Using 409 Conflict in that way might be within your tolerance for bending the rules. I'd certainly not rule it out.
If, however, you're ready to bend that rule, then what's the problem with bending the rule about 500 Internal Server Error?
As I described, you just have to remove or change a single word to make it work.
@Crokus,
I don't think I would return an error
5xxother than infrastructure/exception
Could you please give an example of an infrastructure condition where you would consider a 500 status code to be appropriate?
Likewise, could you please give an example of an exception where you would consider a 500 status code to be appropriate?
In programming and software design, the devil is often in the details, and I do believe that we can uncover some important information if you answer those questions.
It's possible that we'll never agree, but if we reach aporia, I'd at least like to know why. I think I have a pretty good idea about why we're disagreeing, and I'd like to test my hypothesis. You can, of course, also just choose to give up on me 😄
The intent of sending a POST to the /reservations resource is to create a reservation resource--nothing more, nothing less.
If the request is valid, 40x isn't appropriate. 409 would be a situation where the customer already has a reservation and asks to change it (PUT), and another person in their party is on the other line asking to change it at the same time. There is a conflict that the client needs to resolve.
500 works even in the case that it was unexpected. In the real world, there is a query to see if a resevation can be created, so attempting to create a reservation without looking to see if it's available is unexpected.
I see this play out like this:
Customer: "I'd like to make a reservation." Maitre d': "Okay, what time would you like." Customer: "Can you do 7:00 PM?" Maitre d': "Yes." Customer: "Great, put me down." Maitre d': "Well, this is unexpected. My collegue just booked the last table, so I cannot create your reservation."
Client did everything right, server failed. -> 500
@xKloc, not that I disagree, but to illustrate how subtle this kind of discussion is, you can also view /reservations as a (collection) resource that you attempt to change by a POST request to create a new reservation. After all, the entire API models some sort of state, and that state changes if you successfully make a new reservation.
Thus, you could argue that even if you query the state first (as was always my implied intent), a race condition might prevent you from making the reservation. One can, I believe, view that as a conflict.
Can the client resolve that conflict? Perhaps. Maybe by choosing another time, or another day. Maybe by picking a different restaurant.
Decades of software design has taught me that rather than universally right and wrong designs, much depends on the perspective and scope with which one considers a problem.
You are absolutely right. I wasn't speaking in absolutes, but just tackling the question from my perspective. There are different ways to look at everything. It's all about context.
I listened to your talk last night. It was really good. Do you have a Nuget package or recommend one to use for this pattern?
P.S. I've been asking my wife, "But, how do I get the value out of my monad?" in response to everything this morning and she's about to resort to violence.
Do you have a Nuget package or recommend one to use for this pattern?
I'm sorry, you lost me there 😳 Which pattern?
Ah, yes, I usually just write types like Maybe<T> as I need them. They're universal abstractions, so once one understands how they work, they're easy enough to implement.
If you want to expose them via APIs, however, it makes sense to use some common library. There's plenty of NuGet packages around that does that, but I've never used any of them, so I can't endorse any.
No problem, I'll probably write my own. I want to make sure I understand what's happening beneath the hood. Although, It looks pretty straightforward.
Thanks for the tips.