Restored a few error codes and redefined FRAME_ERROR. #521
Conversation
|
It sounds like we're going to be discussing this at the interim. I think that this misses the point of doing a consolidation. For instance, why would anyone actually send HANDSHAKE_TIMEOUT? For that matter, how would they send it? |
|
Good point -- we can remove HANDSHAKE_TIMEOUT. Unless there's a good reason, I'd like to keep the others. I think the errors are useful, and I don't see an advantage to removing the ones that I've listed. |
|
That was an example, there are quite a number more that fit the same criterion. |
|
This might help understand where I'm coming from here. In going through the existing codes, I asked myself the question "what code would I write to handle a CONNECTION_CLOSE/RST_STREAM that contained this error code?" If the code is the same for two error conditions, the separate codes were redundant. The other question to ask might be "why would I send a CONNECTION_CLOSE/RST_STREAM containing this error code?" |
|
I think I understand where you're coming from. That doesn't cover monitoring though; Google monitors these error codes, and we find it useful operationally. I expect that other server will find it useful too. Happy to air this out at the interim for others to weigh in on. |
This is just the error codes, not references to those codes.
One interesting change here is that we don't have an error code for running out of packet numbers. If you think about this for even a few seconds, you will realize that you can't send that error code because it can't go in a packet. And that's even assuming that you manage to keep a connection alive long enough to send that many packets. At 1 million packets a second, that's almost 600 thousand years. Even if we assume that you jump 2^31 packets every time you switch between networks, and change once a minute, that's 16 thousand years. I didn't remove the parenthetical mention of QUIC_NO_ERROR because that is being removed in other pull requests.
martinthomson
force-pushed
the
transport_errors
branch
from
c0a6acf
to
d3dec57
Compare
|
@janaiyengar, I had to rebase the |
This is just the error codes, not references to those codes.
One interesting change here is that we don't have an error code for running out of packet numbers. If you think about this for even a few seconds, you will realize that you can't send that error code because it can't go in a packet. And that's even assuming that you manage to keep a connection alive long enough to send that many packets. At 1 million packets a second, that's almost 600 thousand years. Even if we assume that you jump 2^31 packets every time you switch between networks, and change once a minute, that's 16 thousand years. I didn't remove the parenthetical mention of QUIC_NO_ERROR because that is being removed in other pull requests.
janaiyengar
force-pushed
the
revised_errors
branch
from
c6ebd54
to
4183871
Compare
|
Thanks for the heads-up; rebased. |
There was a problem hiding this comment.
The two issues I feel the strongest on are that I want to tell what frame type caused an error if a frame was invalid, and I want to know about unencrypted stream data. I think the other codes are useful, but the per-frame type codes and the unencrypted data are critical.
draft-ietf-quic-transport.md
Outdated
| @@ -2612,71 +2612,104 @@ CONNECTION_CLOSE or RST_STREAM frame. Error codes share a common code space. | |||
| Some error codes apply only to either streams or the entire connection and have | |||
| no defined semantics in the other context. | |||
|
|
|||
| INTERNAL_ERROR (0x80000001): | |||
| FRAME_ERROR (0x800000TT): | |||
There was a problem hiding this comment.
One nit: Stream frames and acks frames consume multiple spots in the octet, we should define which spots they should use for errors.
draft-ietf-quic-transport.md
Outdated
| INTERNAL_ERROR (0x80000001): | ||
| FRAME_ERROR (0x800000TT): | ||
|
|
||
| : An endpoint received a frame that was badly formatted. For instance, an empty |
There was a problem hiding this comment.
nit: badly -> incorrectly
draft-ietf-quic-transport.md
Outdated
|
|
||
| : An endpoint received a frame that was badly formatted. For instance, an empty | ||
| STREAM frame that omitted the FIN flag, or an ACK frame that has more | ||
| acknowledgment ranges than the remainder of the packet could carry. The final |
There was a problem hiding this comment.
A better ack example would be an ack packet that indicates previously acked packets are now missing.
draft-ietf-quic-transport.md
Outdated
|
|
||
| : A public reset packet was received for this connection post-handshake. | ||
|
|
||
| TOO_MANY_RTOS (0x80000108): |
There was a problem hiding this comment.
This may be useful, but it does assume one implements a too many RTOs feature.
martinthomson
force-pushed
the
transport_errors
branch
from
d3dec57
to
67ccacb
Compare
|
Is this PR obsolete? |
I'd like to go over this again to ensure we don't leave anything out. In the meanwhile, take a look at the direction I'm pushing in.