-
Notifications
You must be signed in to change notification settings - Fork 169
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
Clarify server behavior if content type is not supported #457
Conversation
Part of our problem is that we're already in a situation where JSON is assumed for both input and output. In an ideal world we could be strict about the content types in and out, but I suspect that may create issues with existing implementations. So I would be inclined to tweak this as follows:
This should allow us to retain compatibility with existing implementations that don't provide content information, whilst still allowing SSZ to be made available for individual endpoints. |
Based on my interop testing, all clients provide content type information in both the request and response.
The The existing note also already clarified this and has been added 3 years ago. We might wanna specifically mention that responses should also add a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but its an interesting one.
If clients already don't have 406 implemented, its likely that it'll just be a json response if you request SSZ for example, because they probably dont look at the header anyway...
anyway, i agree with the intent, it's just not that simple to have predictability in this.
yeah, 406 is likely the least supported by clients right now but it's not as important as we can just use a q-value weighted Accept header. It could become more relevant if at some point we wanna adopt SSZ-only clients as suggested by @arnetheduck as you would be able to determine compatibility with beacon node but even there it would be sufficient to check the content type of response. Current state of clients it pretty good though in terms of what's noted in the PR
We might not get to the point where all clients strictly follow this for all APIs, but using tools like Kurtosis it's pretty easy to find compatibility issues. |
This clarifies server behavior if a content type is not supported, allowing clients to handle the error, e.g. by retrying a SSZ request using JSON if the server responds with a 415 status code.
The noted behavior is not specific to the beacon API as it is generally how a well-behaved HTTP server should respond but it becomes especially important here because it allows to gradually adopt SSZ for more routes in a backward compatible way without bumping the API versions.
The most important parts a server needs to support are
Accept
header with multiple entries and quality values (e.g.Accept: application/octet-stream;q=1.0,application/json;q=0.9
) and 415 status code if theContent-Type
header in the request specifies a format that is not supported.Related to #250