Thanks for the additional details :)

Agreed there is ambiguity in the RFC for error codes, and retryability will require additional context either way (idempotent, safe, ..). Some points to consider:
- PROTOCOL_ERROR is allowed as a “catch all”, but the client can’t differentiate from other “real protocol errors” (e.g. incorrect frame format, sequencing, ..) which generally aren’t retryable.
- CANCEL also doesn’t tell the client “why”, but it is known to be application level (as opposed to a protocol issue)

wdyt? Since there is no “right answer” understood if this is deemed undesirable or too risky.

-Scott

> On May 2, 2022, at 4:14 PM, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> Hello!
>
> On Mon, May 02, 2022 at 08:19:44AM -0700, Scott Mitchell via nginx-devel wrote:
>
>> Rational is captured in the patch set below (tests also
>> updated). Please let me know if this is acceptable.
>>
>
>> # HG changeset patch
>> # User Scott Mitchell <scott_mitchell@apple.com>
>> # Date 1651280176 25200
>> # Fri Apr 29 17:56:16 2022 -0700
>> # Node ID ac2a48aa4efb86cd34189dd1b62fe24c5afbab70
>> # Parent a736a7a613ea6e182ff86fbadcb98bb0f8891c0b
>> H2: RST_STREAM(CANCEL) on timeout instead of PROTOCOL_ERROR
>>
>> When a timeout occurs this condition maybe temporary and retryable. For example if the network
>> bandwidth is temporarily throttled. If a PROTOCOL_ERROR is returned the client is unlikely to
>> retry due to unknown error conditions. Returning CANCEL provides more context as an application
>> level error that the client is more likely to retry.
>>
>> diff -r a736a7a613ea -r ac2a48aa4efb src/http/v2/ngx_http_v2.c
>> --- a/src/http/v2/ngx_http_v2.c Tue Feb 08 17:35:27 2022 +0300
>> +++ b/src/http/v2/ngx_http_v2.c Fri Apr 29 17:56:16 2022 -0700
>> @@ -4636,7 +4636,7 @@
>>
>> if (!stream->out_closed) {
>> if (ngx_http_v2_send_rst_stream(h2c, node->id,
>> - fc->timedout ? NGX_HTTP_V2_PROTOCOL_ERROR
>> + fc->timedout ? NGX_HTTP_V2_CANCEL
>> : NGX_HTTP_V2_INTERNAL_ERROR)
>> != NGX_OK)
>> {
>
> Thanks for the patch.
>
> The CANCEL error code is defined by RFC 7540 as follows:
>
> CANCEL (0x8): Used by the endpoint to indicate that the stream is no
> longer needed.
>
> This does not seem to apply to the timeout case. In contrast,
> PROTOCOL_ERROR mostly matches the particular error condition (that
> is, client failed to provide further frames as expected per
> protocol in a reasonable time), and it is also defined to be a
> "catch-all" error:
>
> PROTOCOL_ERROR (0x1): The endpoint detected an unspecific protocol
> error. This error is for use when a more specific error code is
> not available.
>
> As such, PROTOCOL_ERROR looks more appropriate in the particular
> case from the protocol point of view.
>
> If the particular error condition is temporary and retry is
> desired, it is probably something to do per user action, and the
> particular error code shouldn't matter as long as retry is
> requested by the user.
>
> Hope this helps.
>
> --
> Maxim Dounin
> http://mdounin.ru/

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org