Subject: Re: [quicwg/base-drafts] Replace HTTP_MALFORMED_FRAME error code (#2662)

From: Lucas Pardue <[email-masked]>
Date: Mon, 01 Jul 2019 06:42:30 -0700

I've rebased this PR to incorporate all of the error code rationalization changes that were merged to master. This PR does a few things:

* Resurrects H2's FRAME_SIZE_ERROR but broadens it to include frame layout errors - name is HTTP_FRAME_ERROR
* Introduces a new error code HTTP_INVALID_PRIORITY that captures cases where a frame with valid wire layout attempts to create a prioritiation relationship (see additional point below)
* Replaces occurrences of HTTP_MALFORMED_FRAME with: HTTP_ID_ERROR, HTTP_FRAME_ERROR, HTTP_INVALID_PRIORITY.
*  Reshuffles all of the error codes. In doing so, I tried to align the code space with H2 and avoid there being any gaps.

I'm not 100% convinced we need a HTTP_INVALID_PRIORITY code. I'm open to simply using HTTP_FRAME_ERROR.

HTTP_SETTINGS_ERROR and HTTP_MISSING_SETTINGS are candidates to be consolidated (either together, or into another code). This is dependent on discussion in (and related to) https://github.com/quicwg/base-drafts/issues/2783. I suggest we solve that in a different PR.

-- 
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/2662#issuecomment-507269811

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 01 Jul 2019 06:55:17 -0700

@LPardue Thank you for all your effort on refactoring the error codes.

I think my preference goes to merging INVALID_PRIORITY with FRAME_ERROR. The reason is that it is the error code captures a syntactic error of a frame, and because I do not think that each frame should have it's own error code to indicate an syntactic error. As an example, "empty bits" could appear not just in the PRIORITY frame but in any frame. Should we have dedicated error code for frames that contain empty bits? I do not think so. Hence my preference to not have INVALID_PRIORITY.

Regarding rearranging the numbers, I wonder if it would be possible to split the error codes into two groups: connection-level error codes and stream-level ones. In contrary to HTTP/2, most of the HTTP/3 error code are either a connection-level error code or a stream-level error code, not both.

-- 
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/2662#issuecomment-507274879

---

From: Lucas Pardue <[email-masked]>
Date: Mon, 01 Jul 2019 07:14:03 -0700

> I think my preference goes to merging INVALID_PRIORITY with FRAME_ERROR. 
Thanks for the feedback. It's an easy change to make. I'll wait for some supporting or dissenting voices but otherwise will do this.

> Regarding rearranging the numbers, I wonder if it would be possible to split the error codes into two groups: connection-level error codes and stream-level ones. In contrary to HTTP/2, most of the…

This resonates with my earlier analysis. Some of this was captured in https://github.com/quicwg/base-drafts/issues/2816. But I think what you are suggesting is a slightly different thing. Can you explain what splitting might look like and how it would help? I think it gets complicated because 1) we may have a proxy that is trying to translate errors between H2 and H3. 2) stream errors can be upgraded to connection errors

-- 
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/2662#issuecomment-507282456

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 01 Jul 2019 22:54:56 -0700

@LPardue 

:+1:

> This resonates with my earlier analysis. Some of this was captured in #2816. But I think what you are suggesting is a slightly different thing. Can you explain what splitting might look like and how it would help? I think it gets complicated because 1) we may have a proxy that is trying to translate errors between H2 and H3. 2) stream errors can be upgraded to connection errors

IIUC, the following H3 error codes are used at both connection and stream level.

* HTTP_NO_ERROR
* HTTP_GENERAL_PROTOCOL_ERROR
* HTTP_INTERNAL_ERROR

The following codes are used only at connection level, though HTTP_EXCESSIVE_LOAD could be carried at stream level too.
* HTTP_STREAM_CREATION_ERROR
* HTTP_CLOSED_CRITICAL_STREAM
* HTTP_UNEXPECTED_FRAME
* HTTP_FRAME_ERROR
* HTTP_EXCESSIVE_LOAD
* HTTP_WRONG_STREAM
* HTTP_ID_ERROR
* HTTP_INVALID_PRIORITY
* HTTP_SETTINGS_ERROR
* HTTP_MISSING_SETTINGS

The following codes are stream-level errors. While they can be promoted to connection errors, doing so has a risk of resetting other requests inflight. Therefore I'd assume that they would generally be used only at stream-level.
* HTTP_REQUEST_REJECTED
* HTTP_REQUEST_CANCELLED
* HTTP_INCOMPLETE_REQUEST
* HTTP_EARLY_RESPONSE
* HTTP_CONNECT_ERROR
* HTTP_VERSION_FALLBACK

Therefore I thought that it might make sense to explain these three groups differently in the draft.

While I can see the desire to assign codepoints that overlap with HTTP/2, I am not sure if that's a good idea. Not only the error codes are different between H2 and H3, some H2 error codes map to QUIC transport-level errors. Considering the fact that there cannot be a one-to-one mapping, I'm afraid having overlap might backfire against us; it just creates an incentive to pass-through the error codes between different versions of the protocol.

-- 
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/2662#issuecomment-507529717

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 02 Jul 2019 03:57:24 -0700

Thanks for this analysis it does really clarify your position. I think there are two independent discussion points here:

1) should error codes be grouped in a logical order by their intended use?
2) should error codes align with or avoid HTTP/2 where possible?

For (1), I'm not against adopting your suggested order but keen to hear some other opinions here.

> While I can see the desire to assign codepoints that overlap with HTTP/2, I am not sure if that's a good idea. Not only the error codes are different between H2 and H3, some H2 error codes map to Q…

Do you think the incentive grows with increasing number of aligned points? A sledgehammer of a disincentive is to shift these to avoid HTTP/2 completely; e.g. shift by 0x20, 0x50 or 0x100. Note that QPACK codes already start at 0x200

-- 
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/2662#issuecomment-507625776

---

From: ianswett <[email-masked]>
Date: Tue, 02 Jul 2019 07:55:31 -0700

I am not seeing much value in aligning with HTTP/2 error codes.  Alignment would have value if the endpoints could treat a given HTTP/2 and HTTP/3 error code identically.

However, in our current GQUIC implementation, we have a handful of error codes with any special treatment and they're all transport level, so I don't think that's a good argument?

Aligning them has the chance of adding more confusion, so unless there is a technical reason to do so(ie: the above), I think it should not be a goal.

-- 
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/2662#issuecomment-507713784

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 02 Jul 2019 16:42:23 -0700

@LPardue pushed 1 commit.

47b300e4dd5b4a036b7724bba1a2950364ab186d  reduce and upshift error codes

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2662/files/3f1703d612aafe846049539694ceb912ab363c70..47b300e4dd5b4a036b7724bba1a2950364ab186d

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 02 Jul 2019 16:56:15 -0700

dce2938dafb00dc1bf54b3d9fe556a6d0eb3f739  Apply Kazuho's suggested error ordering

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2662/files/47b300e4dd5b4a036b7724bba1a2950364ab186d..dce2938dafb00dc1bf54b3d9fe556a6d0eb3f739

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 02 Jul 2019 16:59:26 -0700

I added two separate commits to shift the error code space to start from 0x100 (plays well with QPACK codes starting 0x200), and to apply @kazuho's proposed ordering. I didn't add any wordsmithing to explain this order as it didn't seem crucial.

-- 
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/2662#issuecomment-507886701

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 02 Jul 2019 19:29:01 -0700

Thank you for the changes.

FWIW, my point was that trying to use the same codepoints for the some errors does not make sense  (I think [@ianswett's comment](https://github.com/quicwg/base-drafts/pull/2662#issuecomment-507713784) captures my feeling). I did not mean that we need to avoid using different block of codepoints than that of HTTP/2.

That said, I would not oppose from starting at 0x100.

-- 
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/2662#issuecomment-507913166

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 17 Jul 2019 03:27:17 -0700

@rmarx suggests that we rename HTTP_INCOMPLETE_REQUEST to HTTP_REQUEST_INCOMPLETE in order to be more consistent with other error codes. That SGTM so I'll make that change unless anyone disagrees.

-- 
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/2662#issuecomment-512197122