Subject: [quicwg/base-drafts] Think carefully before blowing up (#3344)

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Jan 2020 15:46:26 -0800

This is the mildest way to fix #3300.  Cautionary text in the permission to upgrade stream errors to connection errors.  Purely editorial.

Closes #3336, closes #3303.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Think carefully before blowing up

-- File Changes --

    M draft-ietf-quic-http.md (7)

-- Patch Links --

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

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 14 Jan 2020 16:14:28 -0800

LPardue commented on this pull request.

> +An endpoint MAY choose to treat a stream error as a connection error under
+certain circumstances.  Implementations need to consider the impact on
+outstanding requests before making this choice.

LG. 

The other impact of closing conenction with an error relates to Alt-Svc. A client maintaining an Alt-Svc cache may interpret a connetion error as a broken service, which could cause a fallback (e.g. to HTTP/2) and a failure to switch to the alternative until the black mark expires. 

We saw _something_ like this between Chrome and Cloudflare, where one of us was mistakenly detecting something as an error. @RyanAtGoogle might recall.

-- 
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/3344#pullrequestreview-342921580

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 16:53:14 -0800

kazuho approved this pull request.

Thank you for working on the PR. I agree that this level of warning is sufficient.

Client makes the final decision on if a new request should be sent on an existing connection, therefore it is responsible for the outcome of promoting a stream-level error to connection level.

-- 
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/3344#pullrequestreview-342933139

---

From: Martin Thomson <[email-masked]>
Date: Tue, 14 Jan 2020 16:57:42 -0800

martinthomson 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/3344#pullrequestreview-342934405

---

From: ianswett <[email-masked]>
Date: Tue, 14 Jan 2020 17:01:45 -0800

ianswett approved this pull request.

In that case, I believe Chrome was sending a RESET_STREAM after the stream had been closed, which is technically legal, but unnecessary.  And your implementation immediately closed the connection in response to a RESET_STREAM on an already closed stream, causing Chrome to mark QUIC as 'broken' for 5 minutes.

-- 
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/3344#pullrequestreview-342935499

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jan 2020 15:33:54 -0800

martinthomson commented on this pull request.

That case smells like a server bug.  You should not treat receipt of a legal frame as an error.  I assume that the RESET_STREAM was sent only in the case that the stream was fully acknowledged.  Otherwise that might have been a genuine problem, though still not cause for termination.

-- 
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/3344#discussion_r367162515

---

From: Mike Bishop <[email-masked]>
Date: Thu, 16 Jan 2020 12:14:08 -0800

Merged #3344 into master.

-- 
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/3344#event-2955916810