Skip to content

Reproduction of request cancellation in workerd#5062

Open
edmundhung wants to merge 4 commits into
mainfrom
edmundhung/request-cancellation-sample
Open

Reproduction of request cancellation in workerd#5062
edmundhung wants to merge 4 commits into
mainfrom
edmundhung/request-cancellation-sample

Conversation

@edmundhung

Copy link
Copy Markdown
Member

I have been testing request cancellation support in Wrangler and noticed that Workerd does not log the Request was aborted message either. Is that expected?

The same worker does log the message in production.

@edmundhung edmundhung requested review from a team as code owners September 12, 2025 12:14
Comment thread samples/request-cancellation/worker.js Outdated

console.log('Starting long-running task');
// Simulate a long-running task so we can test aborting
await new Promise((resolve) => setTimeout(resolve, 3000));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Try also adding:

    ctx.waitUntil(new Promise((resolve) => setTimeout(resolve, 4000)));

Without the waitUntil(), as soon as the client disconnects, the entire request context is torn down immediately, and hence the abort event doesn't get a chance to be delivered.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just gave that a try in 07ecdbd, but I'm still not seeing any logs.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fwiw, ctx.waitUntil(scheduler.wait(3000)) would do the same thing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't seem to work locally for me either. I'll try putting a debugger on it, unless anyone has any leads.

@jasnell

jasnell commented Sep 16, 2025

Copy link
Copy Markdown
Collaborator

How are you testing the abort? Simply by disconnecting the requester? while the timeout is still pending?

@npaun

npaun commented Sep 16, 2025

Copy link
Copy Markdown
Member

How are you testing the abort? Simply by disconnecting the requester? while the timeout is still pending?

At least that's what I'm doing when trying to repro.

It's a bit of an odd situation. The request is fully sent, and no response is sent at all.

The demos I provided would have a streaming response, and the client would abort while reading, etc.

@jasnell

jasnell commented Sep 16, 2025

Copy link
Copy Markdown
Collaborator

Try starting to stream a response rather than just setting a timeout. Just have it where every second or so it writes a new chunk. Use the IdentityTransformStream to do it... something like...

const { writable, readable } = new IdentityTransformStream();
const writer = writable.getWriter();
const enc = new TextEncoder();
setInterval(() => writer.write(enc.encode('ping')), 1000);
ctx.waitUntil(scheduler.wait(10_000));  // 10 seconds
return new Response(readable);

And see if that makes a difference

@npaun

npaun commented Sep 16, 2025

Copy link
Copy Markdown
Member

@jasnell Yep, that's the "right way" and what I showed in my examples and demos.
But shouldn't @edmundhung's script work too even if it's not a very realistic use of the feature?

@jasnell

jasnell commented Sep 16, 2025

Copy link
Copy Markdown
Collaborator

Yep, it should. If the event is working when data is being streamed but not when we're just waiting for a timeout then that suggests the logic for detecting the disconnect is flawed. Likely it's probably only actually detecting the disconnect when the next chunk of data is being written to the socket.

@npaun

npaun commented Sep 16, 2025

Copy link
Copy Markdown
Member

Yep, it should. If the event it working when data is being streamed but not when we're just waiting for a timeout then that suggests the logic for detecting the disconnect is flawed. Likely it's probably only actually detecting the disconnect when the next chunk of data is being written to the socket.

This seems quite plausible...

@edmundhung

Copy link
Copy Markdown
Member Author

FWIW, I have tried the exact example from the changelog but it doesn't work as well. I just simplified the example here.

@npaun

npaun commented Sep 17, 2025

Copy link
Copy Markdown
Member

@edmundhung I cannot reproduce that locally. I just tried the changlelog example on the latest wrangler and it still works for me.

@edmundhung

Copy link
Copy Markdown
Member Author

@npaun Right. Just test it again and your demo works locally. Thanks for confirming.

@edmundhung edmundhung requested a review from a team as a code owner October 15, 2025 11:27
@edmundhung edmundhung force-pushed the edmundhung/request-cancellation-sample branch from 8c6fe36 to 94ee197 Compare October 15, 2025 14:57
@edmundhung

Copy link
Copy Markdown
Member Author

While working on supporting this in the cloudflare vite plugin, we notice the changelog example doesn't work on Window, as reproduced here:

https://github.com/cloudflare/workerd/actions/runs/18533211919/job/52821326925?pr=5062#step:7:26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants