time out stalled streams, emit stream abort events #2074

turbocrime · 2025-02-22T02:38:05Z

Description of Changes

Streaming responses now use the per-request or transport default timeout configuration to abort a stalled stream.

Related Issue

@penumbra-zone/transport-dom never times out a stalled stream #2073

Checklist Before Requesting Review

I have ensured that any relevant minifront changes do not cause the existing extension to break.

changeset-bot · 2025-02-22T02:38:10Z

🦋 Changeset detected

Latest commit: bf58548

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@penumbra-zone/transport-dom	Patch
minifront	Patch
@penumbra-zone/client	Patch
@penumbra-zone/services	Patch
@penumbra-zone/transport-chrome	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

turbocrime · 2025-02-22T04:36:37Z

packages/transport-dom/src/create.ts

-            { signal },
-          ),
-        ),
+            { signal: AbortSignal.any([signal, chunkDeadline]) },


this maintains the present behavior of signalling the response stream with the original request's abort controller, but also adds the chunk deadline.

to clarify, the AbortSignal.any([signal, chunkDeadline]) will combine the main abort signal with the chunk-level abort signal, and if either is triggered, it cancels the stream and posts an abort message to the port?

turbocrime · 2025-02-22T04:40:37Z

packages/transport-dom/src/create.ts

+      const chunkAc = new AbortController();
+      const chunkDeadline = chunkAc.signal;
+      const chunkDeadlineExceeded = () => {
+        console.debug('chunkDeadlineExceeded');
+        if (timeoutMs) {
+          chunkAc.abort(ConnectError.from('Stream stalled', Code.DeadlineExceeded));
+        }
+      };


once passed to pipeThrough an abort signal cannot be replaced. so i'm using a signal controlled by timeout instead of a bare signal from AbortSignal.timeout, so the timer can be reset at each chunk arrival.

alternatively, an abort signal/controller could be avoided, and setTimeout could be used to call the transformer's cont.error directly.

turbocrime · 2025-02-22T04:42:36Z

packages/transport-dom/src/create.ts

@@ -149,7 +149,7 @@ export const createChannelTransport = ({
    async unary<I extends Message<I> = AnyMessage, O extends Message<O> = AnyMessage>(
      service: ServiceType,
      method: MethodInfo<I, O>,
-      signal: AbortSignal | undefined,
+      signal: AbortSignal | undefined = new AbortController().signal,


the unary and stream methods are updated in the same way.

this just creates a signal that will never abort, if an abort signal is not provided by the caller.

turbocrime · 2025-02-22T21:51:16Z

packages/transport-dom/src/create.ts

+                  signal.addEventListener('abort', () =>
+                    port?.postMessage({ requestId, abort: true } satisfies TransportAbort),
+                  );


like unary requests, this will now also transmit an abort event for streams. this adds abort of pending streams, and could be used to externally abort an established stream.

TalDerei

handling stalled streams is a good improvement 👍

TalDerei · 2025-02-23T02:42:41Z

packages/transport-dom/src/create.ts

-            { signal },
-          ),
-        ),
+            { signal: AbortSignal.any([signal, chunkDeadline]) },


to clarify, the AbortSignal.any([signal, chunkDeadline]) will combine the main abort signal with the chunk-level abort signal, and if either is triggered, it cancels the stream and posts an abort message to the port?

TalDerei · 2025-02-23T02:44:14Z

packages/transport-dom/src/create.ts

@@ -160,10 +160,10 @@ export const createChannelTransport = ({
      const requestId = crypto.randomUUID();

      const requestFailure = new AbortController();
-      const deadline = timeoutMs ? AbortSignal.timeout(timeoutMs) : undefined;
+      const requestDeadline = timeoutMs ? AbortSignal.timeout(timeoutMs) : undefined;


timeoutMs is the same as the default transport timeout in createChannelTransport right?

by default, yes, but it may be zero (indefinite) or a custom value for any individual request

TalDerei · 2025-02-23T14:00:23Z

@turbocrime general question, how do these stream timeouts impact the reliability of background syncing?

turbocrime · 2025-02-26T03:19:48Z

@turbocrime general question, how do these stream timeouts impact the reliability of background syncing?

replied in DM, but repeating here for clarity of discussion:

background syncing itself is not affected. the block processor has its own retry logic, and doesn't rely on this transport.

the exposure of background syncing state in the frontend becomes more fragile, but also the displayed state is more accurate and truthful.

the minifront PR enabling status stream retry is to address the minifront experience #2080

turbocrime · 2025-02-26T05:17:45Z

packages/transport-dom/src/create.ts

+      if (transportFailure.signal.aborted) {
+        throw transportFailure.signal.reason;
+      }


retried streams on a dead transport would create an ever-expanding error message, due to throwIfAborted creating a new error message for each failure. this change will allow the same error to be rethrown.

turbocrime force-pushed the transport-dom-stream-timeout branch 2 times, most recently from bc338d1 to 33cab5f Compare February 22, 2025 03:11

turbocrime force-pushed the transport-dom-headers branch from 5dcd3e5 to 9d32ff2 Compare February 22, 2025 04:28

turbocrime force-pushed the transport-dom-stream-timeout branch from 33cab5f to 568a57e Compare February 22, 2025 04:29

turbocrime commented Feb 22, 2025

View reviewed changes

turbocrime changed the title ~~time out stalled streams~~ time out stalled streams, emit stream abort events Feb 22, 2025

turbocrime mentioned this pull request Feb 22, 2025

transport-dom: improve tests #2078

Merged

TalDerei approved these changes Feb 23, 2025

View reviewed changes

turbocrime force-pushed the transport-dom-headers branch 3 times, most recently from 1824abe to 2c1fc91 Compare February 25, 2025 21:40

turbocrime force-pushed the transport-dom-stream-timeout branch from 155418d to fa86717 Compare February 25, 2025 22:07

turbocrime force-pushed the transport-dom-headers branch 2 times, most recently from 0d95457 to c549a75 Compare February 26, 2025 02:33

turbocrime force-pushed the transport-dom-stream-timeout branch from fa86717 to d68e5fe Compare February 26, 2025 02:33

turbocrime force-pushed the transport-dom-headers branch from c549a75 to 3aa02f6 Compare February 26, 2025 03:21

turbocrime force-pushed the transport-dom-stream-timeout branch from d68e5fe to 98cdcea Compare February 26, 2025 03:22

turbocrime force-pushed the transport-dom-headers branch from 3aa02f6 to cc760d9 Compare February 26, 2025 03:27

turbocrime force-pushed the transport-dom-stream-timeout branch from 98cdcea to a024bb1 Compare February 26, 2025 03:27

turbocrime force-pushed the transport-dom-headers branch from cc760d9 to 7cba666 Compare February 26, 2025 03:57

turbocrime force-pushed the transport-dom-stream-timeout branch from a024bb1 to 4fd6ac3 Compare February 26, 2025 03:58

turbocrime force-pushed the transport-dom-headers branch from 7cba666 to 50e0c6b Compare February 26, 2025 04:18

turbocrime force-pushed the transport-dom-stream-timeout branch from 4fd6ac3 to de77c7c Compare February 26, 2025 04:19

turbocrime force-pushed the transport-dom-headers branch 2 times, most recently from 38a38ff to ece8b54 Compare February 26, 2025 04:47

Base automatically changed from transport-dom-headers to main February 26, 2025 05:13

turbocrime added 4 commits February 25, 2025 21:14

time out stalled streams

17f7b60

emit abort on caller signal for streams

217146f

don't use throwIfAborted

dd8aa46

test status

1952085

turbocrime force-pushed the transport-dom-stream-timeout branch from de77c7c to 1952085 Compare February 26, 2025 05:14

turbocrime mentioned this pull request Feb 26, 2025

minifront: retry status stream #2080

Open

1 task

turbocrime commented Feb 26, 2025

View reviewed changes

changeset typo

bf58548

turbocrime merged commit a11bfe3 into main Feb 26, 2025
1 check passed

turbocrime deleted the transport-dom-stream-timeout branch February 26, 2025 05:26

turbocrime mentioned this pull request Feb 26, 2025

@penumbra-zone/transport-dom never times out a stalled stream #2073

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

time out stalled streams, emit stream abort events #2074

time out stalled streams, emit stream abort events #2074

turbocrime commented Feb 22, 2025 •

edited

Loading

changeset-bot bot commented Feb 22, 2025 •

edited

Loading

turbocrime Feb 22, 2025 •

edited

Loading

TalDerei Feb 23, 2025

turbocrime Feb 22, 2025 •

edited

Loading

turbocrime Feb 22, 2025 •

edited

Loading

turbocrime Feb 22, 2025

TalDerei left a comment

TalDerei Feb 23, 2025

TalDerei Feb 23, 2025 •

edited

Loading

turbocrime Feb 23, 2025

TalDerei commented Feb 23, 2025

turbocrime commented Feb 26, 2025 •

edited

Loading

turbocrime Feb 26, 2025

time out stalled streams, emit stream abort events #2074

time out stalled streams, emit stream abort events #2074

Conversation

turbocrime commented Feb 22, 2025 • edited Loading

Description of Changes

Related Issue

Checklist Before Requesting Review

changeset-bot bot commented Feb 22, 2025 • edited Loading

🦋 Changeset detected

turbocrime Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

TalDerei Feb 23, 2025

Choose a reason for hiding this comment

turbocrime Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

turbocrime Feb 22, 2025 • edited Loading

Choose a reason for hiding this comment

turbocrime Feb 22, 2025

Choose a reason for hiding this comment

TalDerei left a comment

Choose a reason for hiding this comment

TalDerei Feb 23, 2025

Choose a reason for hiding this comment

TalDerei Feb 23, 2025 • edited Loading

Choose a reason for hiding this comment

turbocrime Feb 23, 2025

Choose a reason for hiding this comment

TalDerei commented Feb 23, 2025

turbocrime commented Feb 26, 2025 • edited Loading

turbocrime Feb 26, 2025

Choose a reason for hiding this comment

turbocrime commented Feb 22, 2025 •

edited

Loading

changeset-bot bot commented Feb 22, 2025 •

edited

Loading

turbocrime Feb 22, 2025 •

edited

Loading

turbocrime Feb 22, 2025 •

edited

Loading

turbocrime Feb 22, 2025 •

edited

Loading

TalDerei Feb 23, 2025 •

edited

Loading

turbocrime commented Feb 26, 2025 •

edited

Loading