-
Notifications
You must be signed in to change notification settings - Fork 226
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
httpResponseCode
does not work with short
#1115
Comments
What if we emitted a warning if an |
I think continuing to limit httpResponseCode to integer is a net positive because it removes additional verification implementations need to do when generating code (no need to test with each numeric variant), and integer composes well with intEnum, allowing services to make the responseCode dynamic but still limited to a subset of predefined messages. You don't have that option with short, for example. I think the range of HTTP response status codes is implicit, too. It feels like an unnecessary burden on modelers to make them reify this HTTP restriction in their model, and instead should be left to frameworks and tooling to enforce. |
Oh, TIL about What was the rationale for introducing
So is the intention that the variants of an
How though? I think it should be documented in the spec what happens when the response code falls out of the valid range. That's what I advocated for in #1116, but it got closed arguing that it wasn't necessary because response codes fell within the realms of the HTTP RFC. It seems like with #1254, code generators have the freedom to interpret response codes outside of the valid range prescribed in the HTTP RFC as undefined behavior? I think it'd be nicer if the Smithy spec had a provision telling clients and servers what should they do in these cases, so as to avoid discrepancies among generator implementations. |
intEnum is a subtype of integer, so implementations that don't understand it or special case them treat them as integer. If smithy-rs used the visitors, then it automatically works that way (though it should be updated before smithy-rs 1.0).
intEnum is a subtype of integer. If clients receive a non-modeled integer, they need to be able to carry the value without failing to allow a service to add more values over time. If we supported more numeric types, then we'd have needed to add way more types for numeric types that are seldom used. This would have been confusing for modelers IMO and much more of a burden to implement. More details here https://github.com/awslabs/smithy/blob/main/designs/enum-shapes.md
The values of intEnum aren't auto assigned or anything. They can contain any value and larger values than short.
A 500 makes sense to me. |
HTTP response codes are constrained to the 100-999 inclusive range, according to the RFC. However, you can only apply
httpResponseCode
tointeger
shapes.Ideally we would have
httpResponseCode
work only withshort
.The text was updated successfully, but these errors were encountered: