Subject: [quicwg/base-drafts] Restored a few error codes and redefined FRAME_ERROR. (#521)

From: janaiyengar <[email-masked]>
Date: Thu, 11 May 2017 18:33:48 -0700

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.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/521

-- Commit Summary --

  * Restored a few error codes and redefined FRAME_ERROR.

-- File Changes --

    M draft-ietf-quic-transport.md (69)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/521.patch
https://github.com/quicwg/base-drafts/pull/521.diff

-- 
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/521

---

From: Martin Thomson <[email-masked]>
Date: Thu, 11 May 2017 20:25:47 -0700

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?

-- 
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/521#issuecomment-300974345

---

From: janaiyengar <[email-masked]>
Date: Thu, 11 May 2017 21:30:54 -0700

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.

-- 
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/521#issuecomment-300981005

---

From: Martin Thomson <[email-masked]>
Date: Thu, 11 May 2017 21:45:30 -0700

That was an example, there are quite a number more that fit the same criterion.

-- 
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/521#issuecomment-300982633

---

From: Martin Thomson <[email-masked]>
Date: Thu, 11 May 2017 21:52:10 -0700

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?"

-- 
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/521#issuecomment-300983287

---

From: janaiyengar <[email-masked]>
Date: Thu, 11 May 2017 22:15:25 -0700

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.

-- 
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/521#issuecomment-300985827

---

From: Martin Thomson <[email-masked]>
Date: Thu, 11 May 2017 23:24:17 -0700

@janaiyengar, I had to rebase the `transport_errors` branch, which means that you probably want to rebase this branch too.  `git fetch origin;git checkout revised_errors;git rebase origin/transport_errors` should do the trick.

-- 
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/521#issuecomment-300994212

---

From: janaiyengar <[email-masked]>
Date: Fri, 12 May 2017 15:28:17 -0700

@janaiyengar pushed 1 commit.

9a501be  fix broken things during rebase

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/521/files/4183871492ece12b2e62710ad82c41018622ccdc..9a501beab3d27160501641a6ba7c471453d7065a

---

From: janaiyengar <[email-masked]>
Date: Fri, 12 May 2017 15:30:19 -0700

e407ba7  fix broken things during rebase

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/521/files/9a501beab3d27160501641a6ba7c471453d7065a..e407ba73b275961442f47dd4a6bfc393339aa2d4

---

From: janaiyengar <[email-masked]>
Date: Fri, 12 May 2017 15:31:08 -0700

875675b  fix broken things during rebase

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/521/files/e407ba73b275961442f47dd4a6bfc393339aa2d4..875675b9da255aedbe67f8edf43da087927f1a5b

---

From: janaiyengar <[email-masked]>
Date: Fri, 12 May 2017 15:32:31 -0700

Thanks for the heads-up; rebased.

-- 
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/521#issuecomment-301200540

---

From: ianswett <[email-masked]>
Date: Sun, 14 May 2017 18:24:28 -0700

ianswett commented on this pull request.

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.

> @@ -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):

One nit:  Stream frames and acks frames consume multiple spots in the octet, we should define which spots they should use for errors.

> @@ -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):
+
+: An endpoint received a frame that was badly formatted.  For instance, an empty

nit: badly -> incorrectly

> @@ -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):
+
+: 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

A better ack example would be an ack packet that indicates previously acked packets are now missing.

-NO_ERROR (0x80000003):
+INVALID_VERSION_NEGOTIATION_PACKET (0x80000106):
+
+: Version negotiation packet is malformed.
+
+PUBLIC_RESET_RECEIVED (0x80000107):
+
+: A public reset packet was received for this connection post-handshake.
+
+TOO_MANY_RTOS (0x80000108):

This may be useful, but it does assume one implements a too many RTOs feature.

-- 
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/521#pullrequestreview-37738681