Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions src/QUICConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,18 @@ class QUICConnection {
// No `QUICStream` objects could have been created, however quiche stream
// state should be cleaned up, and this can be done synchronously
for (const streamId of this.conn.readable() as Iterable<StreamId>) {
this.conn.streamShutdown(streamId, quiche.Shutdown.Read, 0);
this.conn.streamShutdown(
streamId,
quiche.Shutdown.Read,
this.reasonToCode('read', e),
);
}
for (const streamId of this.conn.writable() as Iterable<StreamId>) {
this.conn.streamShutdown(streamId, quiche.Shutdown.Write, 0);
this.conn.streamShutdown(
streamId,
quiche.Shutdown.Write,
this.reasonToCode('write', e),
);
}
// According to RFC9000, closing while in the middle of a handshake
// should use a transport error code `APPLICATION_ERROR`.
Expand Down Expand Up @@ -954,8 +962,16 @@ class QUICConnection {
if (quicStream == null) {
if (this[running] === false || this[status] === 'stopping') {
// We should reject new connections when stopping
this.conn.streamShutdown(streamId, Shutdown.Write, 1);
this.conn.streamShutdown(streamId, Shutdown.Read, 1);
this.conn.streamShutdown(
streamId,
Shutdown.Write,
this.reasonToCode('write', errors.ErrorQUICConnectionStopping),
);
this.conn.streamShutdown(
streamId,
Shutdown.Read,
this.reasonToCode('read', errors.ErrorQUICConnectionStopping),
);
continue;
}

Expand Down Expand Up @@ -990,8 +1006,16 @@ class QUICConnection {
if (quicStream == null) {
if (this[running] === false || this[status] === 'stopping') {
// We should reject new connections when stopping
this.conn.streamShutdown(streamId, Shutdown.Write, 1);
this.conn.streamShutdown(streamId, Shutdown.Read, 1);
this.conn.streamShutdown(
streamId,
Shutdown.Write,
this.reasonToCode('write', errors.ErrorQUICConnectionStopping),
);
Comment on lines +1009 to +1013
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the end user ever see the "code" for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe so, yeah. Streams get codes when shutdown. Connections get codes and a message.

this.conn.streamShutdown(
streamId,
Shutdown.Read,
this.reasonToCode('read', errors.ErrorQUICConnectionStopping),
);
continue;
}

Expand Down
5 changes: 5 additions & 0 deletions src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ class ErrorQUICConnection<T> extends ErrorQUIC<T> {
static description = 'QUIC Connection error';
}

class ErrorQUICConnectionStopping<T> extends ErrorQUICConnection<T> {
static description = 'QUIC Connection is stopping';
}

class ErrorQUICConnectionNotRunning<T> extends ErrorQUICConnection<T> {
static description = 'QUIC Connection is not running';
}
Expand Down Expand Up @@ -297,6 +301,7 @@ export {
ErrorQUICServerNewConnection,
ErrorQUICServerInternal,
ErrorQUICConnection,
ErrorQUICConnectionStopping,
ErrorQUICConnectionNotRunning,
ErrorQUICConnectionClosed,
ErrorQUICConnectionStartData,
Expand Down
4 changes: 2 additions & 2 deletions tests/QUICStream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1487,12 +1487,12 @@ describe(QUICStream.name, () => {
// Creating a stream on the server side should throw
const newStream = conn.newStream();
await newStream.writable.close();
const asd = (async () => {
const code = (async () => {
for await (const _ of newStream.readable) {
// Do nothing
}
})();
await expect(asd).rejects.toThrow('read 1');
await expect(code).rejects.toThrow('read 0');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test should be more precise, if you are providing a reasonToCode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a new issue to expand the tests for this.


waitResolveP();
await Promise.all(activeServerStreams);
Expand Down