Subject: [quicwg/base-drafts] Separate HTTP/3 stream errors from connection errors. (#2911)

From: Daan De Meyer <[email-masked]>
Date: Sat, 20 Jul 2019 07:45:59 -0700

With the recent changes in HTTP/3 errors, it becomes possible to completely divide errors into stream errors and connection errors, with each error being either a stream error or connection error. As a result, we can also remove the possibility to treat a stream error as a connection error which allows implementations to determine whether an error is a stream error or a connection error based solely on its type.

As far as I can determine from the draft, only HTTP_STREAM_CREATION_ERROR is used as both a stream error and a connection error. It is only used as a stream error when a peer opens a unidirectional stream of an unknown type. If a new error HTTP_UKNOWN_STREAM_TYPE is split of as a stream error from HTTP_STREAM_CREATION_ERROR, HTTP_STREAM_CREATION_ERROR can be classed as solely a connection error.

[This analysis](https://github.com/quicwg/base-drafts/pull/2662#issuecomment-507529717) from @kazuho further lists HTTP_NO_ERROR and HTTP_GENERAL_PROTOCOL_ERROR as errors that can be both connection and stream errors. I think these can be safely classed as solely connection errors. 

HTTP_NO_ERROR is only defined in the draft for usage in CONNECTION_CLOSE, I'm not sure when it would be used as a stream error (we have more specific error codes to indicate intent when closing a stream).

HTTP_GENERAL_PROTOCOL_ERROR is only used as a stream error in the section discussing malformed requests/responses. I'd suggest using a more descriptive error for this case (HTTP_MALFORMED_MESSAGE  for example) and treating HTTP_GENERAL_PROTOCOL_ERROR as solely a connection error.

An error being either a connection error or a stream error simplifies implementation as they don't have to check on every error if it was communicated as a stream error or a connection error. Upgrading a stream error to a connection error is not a sensible approach for the existing stream errors (for example, closing the connection because of `REQUEST_CANCELLED` does not sound like a reasonable approach). If an implementation chooses to treat a stream error as a connection error, perhaps the INTERNAL_ERROR code should be used instead as not being able to handle a stream error without closing the connection is an implementation issue and should be treated as a bug.

There's probably some things I'm overlooking. Any feedback is welcome.

-- 
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/issues/2911

---

From: Lucas Pardue <[email-masked]>
Date: Sun, 21 Jul 2019 07:21:15 -0700

Partially related issue - https://github.com/quicwg/base-drafts/issues/2816

-- 
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/issues/2911#issuecomment-513558600

---

From: Lucas Pardue <[email-masked]>
Date: Sun, 21 Jul 2019 07:38:55 -0700

One of the thorny points to this discussion is that promotion of stream errors to connection errors is a feature of RFC 7540 Section 5.4.1

> An endpoint can end a connection at any time.  In particular, an
   endpoint MAY choose to treat a stream error as a connection error.

So deviating away from this behaviour means that the feature set diverges, which may add complexity to implmentations that attempt to support both (or propogate between H2 and H3 https://github.com/quicwg/base-drafts/issues/2817).

-- 
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/issues/2911#issuecomment-513560015

---

From: Kazuho Oku <[email-masked]>
Date: Sun, 21 Jul 2019 10:10:40 -0700

@DaanDeMeyer 
> HTTP_NO_ERROR is only defined in the draft for usage in CONNECTION_CLOSE, I'm not sure when it would be used as a stream error (we have more specific error codes to indicate intent when closing a s…

That's a good point. RFC 7540 uses NO_ERROR for instructing the client to stop sending the request when a response is provided early. In HTTP/3, we have created HTTP_EARLY_RESPONSE. But it might be a good idea to use HTTP_NO_ERROR in HTTP/3 too.

> HTTP_GENERAL_PROTOCOL_ERROR is only used as a stream error in the section discussing malformed requests/responses. I'd suggest using a more descriptive error for this case (HTTP_MALFORMED_MESSAGE f…

HTTP_GENERAL_PROTOCOL_ERROR is a catch-all error code that can be for any reason. I'd prefer having that usable at stream-level too.

@LPardue 
> One of the thorny points to this discussion is that promotion of stream errors to connection errors is a feature of RFC 7540 Section 5.4.1

I'd prefer keeping the same design.

I think we need the freedom of promoting stream-level errors to connection errors. For example, when a server detects a suspicious activity by a client at a stream-level, it would make sense to close the connection.

-- 
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/issues/2911#issuecomment-513571273

---

From: Lucas Pardue <[email-masked]>
Date: Sun, 21 Jul 2019 11:21:18 -0700

@kazuho 

> I think we need the freedom of promoting stream-level errors to connection errors. For example, when a server detects a suspicious activity by a client at a stream-level, it would make sense to clo…

I agree. This is a real thing that happens today with H2 and should be maintained.

My question would be, does  it makes sense for promotion to maintain the the error code fidelity? I.e. if you are closing connection on a miss-behaving peer, is there benefit from articulating the specific stream error code, or does it work as well to drop this to HTTP_GENERAL_PROTOCOL_ERROR?

-- 
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/issues/2911#issuecomment-513576272

---

From: Daan De Meyer <[email-masked]>
Date: Sun, 21 Jul 2019 11:23:55 -0700

> HTTP_GENERAL_PROTOCOL_ERROR is a catch-all error code that can be for any reason. I'd prefer having that usable at stream-level too.

PROTOCOL_ERROR sounds to me like something that mandates closing the connection. I do agree with having a catch-all error code for streams as well but I'd just add HTTP_GENERAL_STREAM_ERROR instead of reusing HTTP_GENERAL_PROTOCOl_ERROR. It avoids confusion of whether PROTOCOL_ERROR closed a single stream or the entire connection. 

> I think we need the freedom of promoting stream-level errors to connection errors. For example, when a server detects a suspicious activity by a client at a stream-level, it would make sense to clo…

Why do we need promotion for this? Closing the connection because of suspicious activity at the stream level sounds like a clear use-case for a connection level error such as HTTP_SUSPICIOUS_ACTIVITY (I don't think we should actually add this).

More generically, if an error at the stream level (clearly intended not to close the entire connection since its a stream level error) causes the entire connection to be closed, that should be communicated with a connection level error instead of a stream level error. Many of the current stream level errors don't make any sense at all when used in the context of a connection level error. Instead, if a stream error could possibly mandate closing the entire connection, it should have a corresponding connection level error that more clearly indicates why a stream level error caused the the entire connection to be closed.

-- 
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/issues/2911#issuecomment-513576440

---

From: Lucas Pardue <[email-masked]>
Date: Sun, 21 Jul 2019 11:30:59 -0700

I think what we're missing is that the distinction between connection error and stream "error" is explicitly present in the QUIC transport, by the use of different frames. 

It seems we are discarding that information and then trying to back-infer it from an error code. I'd argue that this is the wrong approach because in practice, there is a good chance the remote peer's approach to selecting error codes is different.

-- 
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/issues/2911#issuecomment-513576904