IPIP-332: Streaming Error Handling on Web Gateways

[hacdias]

See ipfs/kubo#9333 (comment) for more context.

[Jorropo]

I would like to be an options for clients to optout of this behaviour (with a header probably).

Killing the connection can have real performance costs and will require clients to reopen a new connection if they are using HTTP1.1.
If your client as able to detect errors in car files or support trailing headers, you would probably like thoses methods instead of reopenning the connection.

[Jorropo]

Suggested change

	- Sending a `RST_STREAM` (reset stream) frame for HTTP/2
	- Sending a `RST_STREAM` (reset stream) frame for HTTP/2
	- Sending a `CANCEL_PUSH` frame for HTTP/3, see RFC9114 section A.2.5.3.

[marten-seemann]

I haven't read the entire spec, but according to BCP 56, HTTP APIs shouldn't go into this level of detail. HTTP is designed in a way that allows building applications without referring to the specific HTTP version.

[marten-seemann]

@Jorropo Again, haven't read the entire spec, but I are we actually doing server push (which is the only context in which CANCEL_PUSH would be valid)? That feature of HTTP is going to be removed from major browser implementations very soon.

[Jorropo]

I didn't actually red the RFC, I know RST_STREAM on HTTP/2 is what we want, I just CTRL + F RST_STREAM in the HTTP/3 RFC and found this text:

RST_STREAM (0x03): RST_STREAM frames do not exist in HTTP/3, since
QUIC provides stream lifecycle management. The same code point is
used for the CANCEL_PUSH frame (Section 7.2.3).

I guess it should be whatever QUIC's frame is to close a stream unexpectedly.

---

HTTP is designed in a way that allows building applications without referring to the specific HTTP version.

Streaming errors is not a thing HTTP supports, you can setup trailler headers but they aren't supported by browsers.
So we are stealing the rug from under HTTP and use the underlying protocol to generate an unexpected error (which is actually supported and will return errors in all client implementations I've tested), that why we need to use TCP FIN.

I don't know if we should add this to the gateway spec, because most HTTP server implementations give you as much control as the go std does.
How will you implement that on reverse proxies ?

[hacdias]

@marten-seemann as @Jorropo mentioned, the big issue here is that HTTP doesn't have any error handling for streaming. One of my motivations behind this IPIP regards the new TAR format for the gateway (ipfs/kubo#9029).

It is possible that the TAR creation fails while streaming the file to the client due to many reasons. However, if you just stop streaming the TAR, you still get a valid TAR file, but it is incomplete. There's no feedback. Printing a trailer header is useless since browsers are not able to parse them. The only way we found so far to tell the user that something is wrong was by force-closing the HTTP stream.

I also have mixed feelings about having this on the spec since it is so specific. In addition, as @Jorropo mentioned it may be worth it having an opt-out of the behaviour through some header.

[Jorropo]

@hacdias

How will you implement that on reverse proxies ?

I think this is an important question.
I would like to see something like: X-On-Error: reset header sent by the server to indicate this will be done, then a reverse proxy could just not send thoses headers, and clients would be able to tell it is not gonna do this.

[hacdias]

@Jorropo please see the updates I've made to the IPIP.

[marten-seemann]

I haven't read the entire spec, but according to BCP 56, HTTP APIs shouldn't go into this level of detail. HTTP is designed in a way that allows building applications without referring to the specific HTTP version.

[lidel]

Dropping some quick notes:

• Agree with Marten, HTTP API should not go into low level connection detail. We want gateway interface to be future-proof.
• The more gateway implementations exist, the less control we have over these low level details. Need to solve this on higher client application layer.

👉 due to this I am closing this IPIP

Instead, suggest opening a new IPIP with simpler proposal (but looking for better ones! – comment below):

• The problem is that there is no way to signal that mid-way transfer error occurred. Instead of trying to bolt-on something on the server side, instead, spec should specify inexpensive way how clients should detect that the entire CAR got transferred:

• Introduce optimization where we always add a zero-length raw block at the end of a CAR as a tombstone indicator – if this well-known CID is present, the client can skip hashing the last block, because we know the last block arrived safely.

• If tombstone is not present, then the client MUST verify hash of the last block. If hash does match, means the connection was not interrupted mid-transfer (downside is additional CPU cost due to hashing that one block, that is why we prefer to have tombstone)

The tombstone is backward-compatible, and allows us to have an escape hatch to skip hashing and save CPU.

[lidel]

I know this was opt-in, but as usual, reminding that Trailers can't be read by JS in web browsers: mdn/browser-compat-data#14703 (lesson learned from Kubo RPC: ipfs/js-ipfs#2519)
Gateway specs should never require things that are not supported in browsers, as this leads to the very problem types this IPIP aims to solve.

[jbenet]

#332 (review)

• null objects are a very nasty way to go.
• having a tombstone (ie some car files have it and some dont) is super messy and likely to lead to mysterious errors.
• it also opens a surface area of attack: now everyone has to check that we arent linking to the null object from some object, because if we are, then it will terminate a car stream early, and we have to do that in a ton of places (think the lovely wonders of \0 in string operations). this is really nasty and you can guarantee that it will not be running _ correctly everywhere_, so you can guarantee that it will be run incorrectly somewhere, and also have to deal with that.
• null termination is just a world of pain, and why ipfs went the way of self-describing objects. usually, most problems can indeed be turned into self-describing structures, even streams of unknown length. -- this is usually done by introducing a wrapper object that contains the information you want.
• for example, define a format like a car-stream that includes a header in between every object (similar to tar) and that header can signal the end of a car-stream. you would have a 1-1 from any car to any car-stream, and make very explicit what is data and what is control plane.

[jbenet]

• more succinctly: i think you want a "content addressed stream" (CAS), not a "content addressed archive" (CAR).
• separately, this thread reminded me that we're likely due for a CARv3 with (a) support for explicit finite car files (ie the object count in the header), and (b) verifiable header (explicit roots in the header (as in v1) + a hash of the v2 index objects) -- such that a single hash (of a single small object) can be used to verify the entire thing (the original property CARs were invented for)

[rvagg]

we're likely due for a CARv3 with (a) support for explicit finite car files (ie the object count in the header), and (b) verifiable header (explicit roots in the header (as in v1) + a hash of the v2 index objects) -- such that a single hash (of a single small object) can be used to verify the entire thing (the original property CARs were invented for)

Happily I think we can mush all of that into CARv2 without breaking. We'd just need that as a wishlist and some consumers of that functionality to help drive a spec to lock it in.

• feat: EmbedMessage WrapV1 option & Reader#ReadEmbeddedMessage ipld/go-car#322
• feat: add carv2-messaging example for embedding messages in CARv2 ipld/js-car#89