Subject: [quicwg/base-drafts] Remove error code and reason phrase from GOAWAY (#355)

From: Martin Thomson <[email-masked]>
Date: Wed, 01 Mar 2017 19:33:10 -0800

In an error condition, CONNECTION_CLOSE is your friend.

Closes #352. Builds on #354.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * GOAWAY needs a maximum stream number in both directions
  * Advice on use of GOAWAY during error handling
  * Remove error code and reason phrase from GOAWAY

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/355.patch
https://github.com/quicwg/base-drafts/pull/355.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/355

---

From: Lucas Pardue <[email-masked]>
Date: Thu, 02 Mar 2017 09:05:17 -0800

LPardue commented on this pull request.

-* Reason Phrase Length: A 16-bit unsigned number specifying the length of the
-  reason phrase.  This may be zero if the sender chooses to not give details
-  beyond the error code.
+Last Server Stream ID:
+
+: The last server-initiated Stream ID which was accepted by the sender of the

"The last server-initiated Stream ID which was accepted by the sender of the GOAWAY frame" sounds a bit wonky. 

-- 
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/355#pullrequestreview-24786430

---

From: ianswett <[email-masked]>
Date: Thu, 02 Mar 2017 09:14:07 -0800

ianswett commented on this pull request.

Yes, how about was created or will be created?

-- 
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/355#discussion_r103978332

---

From: Mike Bishop <[email-masked]>
Date: Thu, 02 Mar 2017 14:16:59 -0800

MikeBishop commented on this pull request.

Because the sender isn't necessarily the server, and the client isn't "creating" server-initiated streams.  Maybe we could say "the highest ... ID on which the sender has processed or will process data"?  But that doesn't seem more compact, so I'm not sure it's an improvement.

But regardless, that's feedback on #354, not this change in particular.

-- 
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/355#discussion_r104044159

---

From: Martin Thomson <[email-masked]>
Date: Thu, 02 Mar 2017 15:38:23 -0800

martinthomson commented on this pull request.

I have taken this feedback in and will update #354 with this.  I found something else too.

-- 
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/355#discussion_r104058335

---

From: ianswett <[email-masked]>
Date: Fri, 03 Mar 2017 06:01:26 -0800

-* Error Code: A 32-bit field error code which indicates the reason for closing
-  this connection.
+Last Client Stream ID:

I might be biased towards Largest, instead of Last

> +exchanged.  An endpoint MUST set the value of the Last Client or Server Stream
+ID to be at least as high as the highest-numbered stream on which it either sent
+data or received and delivered data to the application protocol that uses QUIC.
+
+An endpoint MAY choose a larger stream identifier if it wishes to allow for a
+number of streams to be created.  This is especially valuable for peer-initiated
+streams where packets creating new streams could be in transit; using a larger
+stream number allows those streams to complete.
+
+In addition to initiating a graceful shutdown of a connection, GOAWAY MAY be
+sent immediately prior to sending a CONNECTION_CLOSE frame that is sent as a
+result of detecting a fatal error.  Higher-numbered streams than those indicated
+in the GOAWAY frame can then be retried.
+
+For peer-initiated streams, an endpoint might indicate a lower value for the
+highest stream number than the value that might be sent by a peer.  After

I don't think sending a GOAWAY in response does anything unless the peer receiving the GOAWAY wants to further decrease one or both of the Last Stream IDs, so typically I wouldn't expect this is necessary.  As such, I'd recommend changing this to MAY and explain it's only if the Last Stream IDs are too large.

-- 
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/355#pullrequestreview-24976696

---

From: Martin Thomson <[email-masked]>
Date: Sun, 05 Mar 2017 14:34:35 -0800

I've kept this as a SHOULD but added the requested conditions.  It's important that "safe" retries are enabled as much as possible and failure to echo will create unnecessary uncertainty.  This is a big deal in HTTP.

-- 
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/355#discussion_r104326703

---

From: ianswett <[email-masked]>
Date: Sun, 05 Mar 2017 14:40:31 -0800

You have to ack the GOAWAY, so why echo it, that would seem redundant.

-- 
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/355#discussion_r104326885

---

From: Martin Thomson <[email-masked]>
Date: Sun, 05 Mar 2017 14:56:21 -0800

Actually, now that I think about it, the only reason why you would reduce the largest stream number is because you didn't use those streams, and if you don't want to use those streams, then you can just *not use them*.  Let's remove this paragraph.

-- 
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/355#discussion_r104327343

---

From: ianswett <[email-masked]>
Date: Mon, 06 Mar 2017 08:43:58 -0800

ianswett approved this pull request.

Maybe GOAWAY is going away, but in the meantime, this looks good.

-- 
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/355#pullrequestreview-25299894

---

From: janaiyengar <[email-masked]>
Date: Wed, 08 Mar 2017 11:26:41 -0800

janaiyengar approved this pull request.

-- 
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/355#pullrequestreview-25860810

---

From: janaiyengar <[email-masked]>
Date: Wed, 08 Mar 2017 11:26:58 -0800

Merged #355.

-- 
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/355#event-991920711