Fix #182: Convert error codes before passing them to QUIC

[barafael]

No description provided.

[BiagioFesta]

Thank you for taking care of this!

Is the reverse map necessary when receiving the code from stream's peer?

[barafael]

I have no idea! Sounds like yes it is?

[finnbear]

Upon reading all relevant documentation, it appears that the conversion is only for WT application error codes and not any of the existing ErrorCode variants, all of which are in the space of valid HTTP/3 error codes.

This can be seen as follows:

• Most of the variants are in the h3_error_codes module and numerically match HTTP/3 error code entries in https://datatracker.ietf.org/doc/html/draft-ietf-quic-http#iana-error-table
• For the variant in the qpack_error_codes namespace, it numerically matches an HTTP/3 error code entry in https://www.rfc-editor.org/rfc/rfc9204.html
• For the variants in the wt_error_codes module, they numerically match HTTP/3 error code entries in https://ietf-wg-webtrans.github.io/draft-ietf-webtrans-http3/draft-ietf-webtrans-http3.html#name-http-3-error-code-registrat

In other words, if we wanted application error codes, they would work like this:

enum ErrorCode{
    Datagram,
    NoError,
    ...
    Application(u8), // Edit: should be u32
}

impl ErrorCode {
    pub fn to_code(self) -> VarInt {
        match self {
            ErrorCode::Datagram => h3_error_codes::H3_DATAGRAM_ERROR,
            ErrorCode::NoError => h3_error_codes::H3_NO_ERROR,
            ...
            ErrorCode::Application(n) => to_http3(n),
        }
    }
}

But I'm not sure Application would be a useful variant. Instead, I suggest looking into whether reset(error_code) should be a u8 (edit: u32) and go through the to_http3 conversion:

wtransport/wtransport/src/stream.rs

Lines 83 to 93 in 4fcb949

	/// Closes the send stream immediately.
	///
	/// No new data can be written after calling this method. Locally buffered data is dropped, and
	/// previously transmitted data will no longer be retransmitted if lost. If an attempt has
	/// already been made to finish the stream, the peer may still receive all written data.
	///
	/// If called more than once, subsequent calls will result in `StreamWriteError::Closed`.
	#[inline(always)]
	pub fn reset(&mut self, error_code: VarInt) -> Result<(), StreamWriteError> {
	self.0.reset(error_code)
	}

Likewise, consider converting VarInt to u8 (edit: u32) or Option<u8> (edit: Option<u32>), the result of a (potentially fallible) from_http3 conversion:

wtransport/wtransport/src/error.rs

Lines 157 to 159 in 4fcb949

	/// The peer is no longer accepting data on this stream.
	#[error("stream stopped (code: {0})")]
	Stopped(VarInt),

(or, if non-application error codes are expected, consider exposing pub fn application_error_code(VarInt) -> Option<u8> so users can make sense of the error code).
(or, having a separate StreamWriteError variant for successfully converted application error codes)
(or, using Option<ErrorCode> instead of VarInt for Stopped)

[finnbear]

There is one thing I'm unsure about:

• https://ietf-wg-webtrans.github.io/draft-ietf-webtrans-http3/draft-ietf-webtrans-http3.html#name-resetting-data-streams implies there are 2^32 web transport application errors
• https://www.ietf.org/archive/id/draft-ietf-webtrans-http3-02.html#name-http-3-error-code-registrat implies there are only 2^8 web transport application errors

I assume this is because these are two different versions of the spec.

Edit: Ah, it changed from 2^8 to 2^32 between drafts 5 and 6 (draft N can be obtained at https://www.ietf.org/archive/id/draft-ietf-webtrans-http3-0N.html#name-resetting-data-streams)

[MOZGIII]

I have no idea! Sounds like yes it is?

It definitely is

[MOZGIII]

Check my implementation at https://github.com/MOZGIII/xwt/blob/ffc8b2cc135acaf7f5229a01775e3da92a22684e/crates/xwt-wtransport/src/lib.rs#L284-L323

Might be relevant for #211 (comment)

[finnbear]

Might be relevant for #211 (comment)

Can confirm; you use the conversion for application error/close codes, which is correct. This conversion could be moved into wtransport.

[MOZGIII]

Let's do it! I'm busy with building the reset support Web part of the xwt now, but feel free to port my code from that PR to wtransport