Re: [quicwg/base-drafts] Split error code space (#722)

[janaiyengar <[e-mail]>]

janaiyengar commented on this pull request.

Comments inline. Can you also please not stack these PRs? Github is really difficult to use for review of large changes like this one, and stacking PRs makes it even harder. I understand the pains of merge conflicts, but honestly, that's a one-time pain during merge, not a continual pain during review.

>  
-The portion of the QUIC error code space allocated for the crypto handshake is
-0xC0000000-0xFFFFFFFF. The following error codes are defined when TLS is used
-for the crypto handshake:
+This document defines error codes from the error code space used in
+{{QUIC-TRANSPORT}}.

This document --> This section.
Also, editorial nit: I like the mention of "crypto handshake when TLS is used for it".

>  
 Error Code:
 
 : A 32-bit error code which indicates the reason for closing this connection.
+  TRANSPORT_CLOSE uses codes from the space defined in {{error-codes}};
+  APPLICATION_CLOSE uses codes from the application protocol error code space
+  ({{app-error-codes}}).

Seems unnecessary to mention APPLICATION_CLOSE in the definitoion of TRANSPORT_CLOSE.

> @@ -2608,11 +2654,10 @@ discarded upon receipt.  This avoids potential ambiguity about which STREAM
 frames count toward flow control.
 
 Upon receipt of a STOP_SENDING frame on a stream in the "open" or "half-closed
-(remote)" states, an endpoint MUST send a RST_STREAM with an error code of
-QUIC_RECEIVED_RST.  If the STOP_SENDING frame is received on a stream that is
-already in the "half-closed (local)" or "closed" states, a RST_STREAM frame MAY
-still be sent in order to cancel retransmission of previously-sent STREAM
-frames.
+(remote)" states, an endpoint MUST send a RST_STREAM frame.  If the STOP_SENDING

What's the proposed error code in the RST_STREAM now? If there's a requirement for QUIC to generate and send this frame, the error code has to be specified.

>  
-An endpoint that receives an invalid CONNECTION_CLOSE frame MUST NOT signal the
-existence of the error to its peer.
+An endpoint that chooses not to retransmit packets containing TRANSPORT_CLOSE or
+APPLICATION_CLOSE risks a peer missing the first such packet.  The only
+mechanism available to an endpoint that continues to receive data for a
+terminated connection is to use the stateless reset process
+({{stateless-reset}}).

editorial nit: I would merge this para with the one above, since this one follows from the earlier.

>  
-An endpoint that receives an invalid CONNECTION_CLOSE frame MUST NOT signal the
-existence of the error to its peer.
+An endpoint that chooses not to retransmit packets containing TRANSPORT_CLOSE or
+APPLICATION_CLOSE risks a peer missing the first such packet.  The only

Not just the first packet, but any packets lost in the network.

> @@ -2923,115 +2975,102 @@ Stream 0 is critical to the functioning of the entire connection.  If stream 0
 is closed with either a RST_STREAM or STREAM frame bearing the FIN flag, an
 endpoint MUST generate a connection error of type PROTOCOL_VIOLATION.
 
-Some application protocols make other streams critical to that protocol.  An
-application protocol does not need to inform the transport that a stream is
-critical; it can instead generate appropriate errors in response to being
-notified that the critical stream is closed.
-
-An endpoint MAY send a RST_STREAM frame in the same packet as a CONNECTION_CLOSE
-frame.
-
-
-## Error Codes
+A stream error is always an application-layer construct.  A RST_STREAM MUST NOT

I would drop this first sentence -- it's a bit imprecise and does not add value.

> -Some application protocols make other streams critical to that protocol.  An
-application protocol does not need to inform the transport that a stream is
-critical; it can instead generate appropriate errors in response to being
-notified that the critical stream is closed.
-
-An endpoint MAY send a RST_STREAM frame in the same packet as a CONNECTION_CLOSE
-frame.
-
-
-## Error Codes
+A stream error is always an application-layer construct.  A RST_STREAM MUST NOT
+be generated by the QUIC transport layer.  Resetting a stream at the transport
+layer could cause unrecoverable loss of application protocol state.  Application
+protocols might require certain streams to be reliably delivered in order to
+guarantee consistent state between endpoints.  For this reason, all errors that
+can be detected at the transport layer result in connection errors.

This last sentence does not belong in this section, and is unnecessary. I would drop it.

>  
-Error codes are 32 bits long, with the first two bits indicating the source of
-the error code:
+An endpoint that detects a stream error MAY choose to treat the error as a
+connection error and send an APPLICATION_CLOSE frame in place of RST_STREAM.

This is confusing. What is the "endpoint"? QUIC, or HTTP/QUIC? If QUIC is no longer generating RST_STREAM, then which stream errors are detected by QUIC? I suspect the answer is that the endpoint is HTTP/QUIC, in which case I would remove this sentence from here entirely.

>  
 : An endpoint detected an error in a specific frame type.  The frame type is
   included as the last octet of the error code.  For example, an error in a
-  MAX_STREAM_ID frame would be indicated with the code (0x80000106).
+  MAX_STREAM_ID frame would be indicated with the code (0x106).
+
+See {{iana-error-codes}} for details of registering new error codes.
+
+
+## Application Protocol Error Codes {#app-error-codes}
+
+Application protocol error codes are 32-bits long, but the management of
+application error codes are left to application protocols.  Application protocol
+error codes are used for the RST_STREAM ({{frame-rst-stream}}) and
+APPLICATION_CLOSE ({{frame-application-close}}) frames.
+
+Application protocols SHOULD define an error codes for indicating no error and

typo: remove "an"

>  
 : An endpoint detected an error in a specific frame type.  The frame type is
   included as the last octet of the error code.  For example, an error in a
-  MAX_STREAM_ID frame would be indicated with the code (0x80000106).
+  MAX_STREAM_ID frame would be indicated with the code (0x106).
+
+See {{iana-error-codes}} for details of registering new error codes.
+
+
+## Application Protocol Error Codes {#app-error-codes}
+
+Application protocol error codes are 32-bits long, but the management of

16-bits

> @@ -1596,7 +1622,7 @@ The RST_STREAM frame is as follows:
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                        Stream ID (32)                         |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-|                        Error Code (32)                        |
+| App Protocol Error Code (16)  |

This is still 32 bits in this PR, right?

> @@ -2923,115 +2975,102 @@ Stream 0 is critical to the functioning of the entire connection.  If stream 0
 is closed with either a RST_STREAM or STREAM frame bearing the FIN flag, an
 endpoint MUST generate a connection error of type PROTOCOL_VIOLATION.
 
-Some application protocols make other streams critical to that protocol.  An
-application protocol does not need to inform the transport that a stream is
-critical; it can instead generate appropriate errors in response to being
-notified that the critical stream is closed.
-
-An endpoint MAY send a RST_STREAM frame in the same packet as a CONNECTION_CLOSE
-frame.
-
-
-## Error Codes
+A stream error is always an application-layer construct.  A RST_STREAM MUST NOT
+be generated by the QUIC transport layer.  Resetting a stream at the transport

drop "transport layer". 

Also, it's odd to say QUIC MUST not generate a RST_STREAM, since it is a QUIC frame and QUIC does in fact generate it. How about "RST_STREAM MUST be instigated by the application and MUST carry an application error code."?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/722#pullrequestreview-56230010