Return a more specific http status code on conflict.#10207
Return a more specific http status code on conflict.#10207
Conversation
|
This is going to break a whole lot of tests. |
sudomateo
left a comment
There was a problem hiding this comment.
Usually, generic HTTP status codes are used as a security measure to prevent clients from enumerating information about API objects. Since we already return ObjectAlreadyExists text perhaps that's a moot point and this change makes sense as-is.
d0c5fc8 to
b77feb2
Compare
|
Reference: https://github.com/oxidecomputer/omicron/blob/main/docs/http-status-codes.adoc#client-errors This is changeable but it does seem like a breaking API change. |
Yes, but mostly the kind of test that Claude can fix.
I think that's right. I don't think the choice of the generic 400 over a 409 here was related to obfuscation, because creating a resource would be behind auth, so a malicious user would hit a 401 or 403 before being able to enumerate resources.
Yes, it's possible that customers are depending on receiving a 400, although I'm not sure how likely that is. How disruptive would this change be? All things being equal, I think it would be nice to use more specific http status codes, but if it would be annoying to us or users, I'll just close this. |
|
Honestly not sure. I agree 409 is more correct, but the improvement is small enough that it would only have to be a tiny bit inconvenient to not be worth it. There are a couple of spots in the console where we look at the status code, but I think we only rely on it for 404 and 403, not for 400, precisely because the latter is so general that the error code is always more informative. |
Just a nit I noticed while working on something else. Semantically,
ObjectAlreadyExistsmaps to an http 409 and not a 400.