openid
Update error table
A follow-up from https://github.com/openid/sharedsignals/pull/247
@jischr please take a look
Returning a 400 if the delivery method is not supported. I looked through the 4xx error codes. Can we suggest providing a reason phrase with the code to delineate why it's a bad request? Thoughts @jischr
Returning a 400 if the delivery method is not supported. I looked through the 4xx error codes. Can we suggest providing a reason phrase with the code to delineate why it's a bad request? Thoughts @jischr
So this is a bug in the current spec?
Would using HTTP status code 406 be better or applicable?
406 - Not Acceptable When making a request, the client can indicate to the web server what kind if data it will accept back. The header with the 406 error code is returned if the web server detects that the only response it can generate and return to the client is not acceptable by the client. This error occurs very rarely with web browsers, because most clients accept any data returned from the server.
The current language says "errors are reported...", which makes the current behavior the equivalent of a "MUST", changing this will break backward compatibility. There is no evidence right now that this is an implementation blocker, so we can review this after v1Final.
The current language says "errors are reported...", which makes the current behavior the equivalent of a "MUST", changing this will break backward compatibility. There is no evidence right now that this is an implementation blocker, so we can review this after v1Final.
Not sure I understand your comment @atultulshi , this PR was to bring alignment of the text which describes the error and the table which lists these scenarios.
The current language says "errors are reported...", which makes the current behavior the equivalent of a "MUST", changing this will break backward compatibility. There is no evidence right now that this is an implementation blocker, so we can review this after v1Final.
Not sure I understand your comment @atultulshi , this PR was to bring alignment of the text which describes the error and the table which lists these scenarios.
Good point. I think in the discussion around this PR we confused the topics of requesting verification events when the stream was paused or disabled. We decided to keep that one out of the scope of v1Final. I won't be there, but you can discuss this PR separately in the meeting on 4/29.
@appsdesh The text says that the Transmitter MAY send the 400 code if the delivery method is not available. But the intro to the table says "Errors are signaled" which should be understood more like a MUST. Adding the delivery unavailability to the table would break backwards compatibility.
The solution might be to go through the spec and reword the error tables so that they say MAY in the intro. But that seems contentious enough to push off until after v1.
@appsdesh The text says that the Transmitter MAY send the 400 code if the delivery method is not available. But the intro to the table says "Errors are signaled" which should be understood more like a MUST. Adding the delivery unavailability to the table would break backwards compatibility.
The solution might be to go through the spec and reword the error tables so that they say MAY in the intro. But that seems contentious enough to push off until after v1.
We are creating two sources of truth for the error codes, one in the table and another buried in the description. I understand MAY vs MUST, but we are likely making it difficult for the implementers to see a list of all expected/suggested errors. We should align on this holistically for v1 and not punt it.