ietf-wg-gnap / gnap-core-protocol

146 stars 26 forks source link

Incorrect example at the bottom of section 5.2. #491

Closed blue-ringed-octopus-guy closed 1 year ago

blue-ringed-octopus-guy commented 1 year ago

At the bottom of section 5.2. of the document, it has the following example:

For example, if the client instance has polled too many times before the RO has approved the request, the AS would respond with a message like this:

{
    "error": "too_many_attempts"
}
Since this response does not include a continue section, the client instance cannot continue to poll the AS for additional updates and the grant request is finalized. If the client instance still needs access to the resource, it will need to start with a new grant request.

This example doesn't seem to be appropriate or accurate for two reasons:

Firstly, in this scenario it is talking about the client polling the AS to find out if a request has been authorized. In this scenario there is no overall limit of attempts that the client can poll, but there is a "wait" value defined. If the client ignores the "wait" value then the AS should return the "too_fast" error code to the client, not "too_many_attempts". Therefore, the example above is incorrect.

Secondly, it doesn't seem appropriate for the AS to move the grant request to the "finalized" state and force the client to restart the process every time a "too_fast" error occurs. The AS MAY choose to do this after multiple occurrences of the "wait" value being ignored by a particular client, but I don't think it would be right for an AS to assume that the client is a bad actor or badly behaved if there is only one occurrence of this happening. Therefore, the example shown above appears to be slightly misleading.

jricher commented 1 year ago

I believe the error and example are correct for what's being shown here.

This scenario is separate from the too_fast error condition and is explicitly about how many times the client has polled, not how quickly. The AS is allowed to limit the total number of polls, if it chooses, and this is the error that would communicate that back to the client.

blue-ringed-octopus-guy commented 1 year ago

If this is the case then how would the AS indicate to the client what the limit is for the number of times it is allowed to poll? There doesn't appear to be a field in the specification that would allow this? I therefore still think this example is misleading.

jricher commented 1 year ago

You are correct, there is not a field that gives a hint to the client that such a limit is in place. The thing is, that limit could be a static number of times, or some variable threshold, or any other mechanism that the AS decides on. There's not a lot of good information that a client would be able to act on.