-
Notifications
You must be signed in to change notification settings - Fork 79
Cancellation primitive #3032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Cancellation primitive #3032
Conversation
79934de to
0431e29
Compare
|
This may interest @gibson042. By way of disclosure, I did make extensive use of Claude 4.5 Opus to produce this very quickly, and watched it closely. This is produced more as an exercise to validate that the stochastic parrot might make some exercises, otherwise hard to get around to, more achievable in short order. |
30549aa to
0431e29
Compare
|
Aside, I noticed an opportunity to use makePromiseKit, but that exercise is dashed because that produces a hardened promise, and so we cannot introduce a new |
…led property behind
da7e6f1 to
f405eba
Compare
erights
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please look at #3035 . It is a failed experiment for a different approach -- use a static function rather than a getter. But it has a fatal security flaw that it explains. In any case, the exercise gave me a full understanding of yours. I'm ready to approve, but only after discussing #3035.
This is a cool project, thanks!
|
Another consideration we should contemplate: We should also think about graceful shutdown. There are cases where |
rekmarks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good! Some non-blocking feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this warrants a couple of test cases in test/safe-promise.test.js.
| const { cancelled, cancel } = makeCancelKit(); | ||
|
|
||
| // Check synchronously if cancelled | ||
| console.log(cancelled.cancelled); // undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not false?
| const { cancelled: childCancelled } = makeCancelKit(parentCancelled); | ||
|
|
||
| cancelParent(Error('Parent cancelled')); | ||
| // childCancelled is now also cancelled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // childCancelled is now also cancelled | |
| console.log(childCancelled.cancelled) // true |
| ## TypeScript Types | ||
|
|
||
| ```ts | ||
| // The cancellation token type | ||
| type Cancelled = Promise<never> & { readonly cancelled: undefined | true }; | ||
|
|
||
| // The cancel function type | ||
| type Cancel = (reason?: Error) => void; | ||
|
|
||
| // The result of makeCancelKit() | ||
| type CancelKit = { | ||
| cancelled: Cancelled; | ||
| cancel: Cancel; | ||
| }; | ||
|
|
||
| // Callback signature for allMap and anyMap | ||
| type CancellableCallback<T, R> = ( | ||
| value: T, | ||
| index: number, | ||
| cancelled: Cancelled | ||
| ) => R | Promise<R>; | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## TypeScript Types | |
| ```ts | |
| // The cancellation token type | |
| type Cancelled = Promise<never> & { readonly cancelled: undefined | true }; | |
| // The cancel function type | |
| type Cancel = (reason?: Error) => void; | |
| // The result of makeCancelKit() | |
| type CancelKit = { | |
| cancelled: Cancelled; | |
| cancel: Cancel; | |
| }; | |
| // Callback signature for allMap and anyMap | |
| type CancellableCallback<T, R> = ( | |
| value: T, | |
| index: number, | |
| cancelled: Cancelled | |
| ) => R | Promise<R>; | |
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mainly seems to restate the readme. Delete?
Description
This PR introduces a new
@endo/cancelpackage that providesPromise<never>-based cancellation tokens with synchronous observation capability, designed for use with Endo and CapTP.Core API
makeCancelKit(parentCancelled?)- Creates a{ cancelled, cancel }pair wherecancelledis aPromise<never>with a synchronous.cancelledgetter. Supports hierarchical cancellation via optional parent token.Operators
allMap(values, fn, parentCancelled)- Maps over values with cancellation propagation; cancels all on first rejectionanyMap(values, fn, parentCancelled?)- Races cancellable jobs; cancels remaining after first successdelay(ms, parentCancelled)- Cancellable delay using ambientsetTimeoutmakeDelay(setTimeout, clearTimeout)- Factory fordelaywith injectable timer functions (fromdelay-lite)Web API Integration
toAbortSignal(cancelled)- ConvertsCancelled→AbortSignalfor use withfetchand other web APIsfromAbortSignal(signal)- ConvertsAbortSignal→Cancelledfor integrating web cancellation sourcesAdoption in Daemon and CLI
This PR also refactors
@endo/daemonand@endo/clito usemakeCancelKitinstead of the previous pattern of usingmakePromiseKitfor cancellation. This demonstrates the ergonomic improvements and validates the design against real-world usage.Pass-Style Relaxation
This PR includes a surgical change to
@endo/pass-stylethat allows promises with acancelledgetter to be passable.Previously,
safe-promise.jsrejected any promise with string-named own properties. This change adds a specific exception forcancelled:Constraints enforced:
Why this is safe:
"&0")cancelledproperty is naturally omitted during serialization—it never crosses the wireRationale:
Cancelledtokens (promises with.cancelledgetter) to be passed as arguments to remote methodsSecurity Considerations
harden().cancelledgetter is intentionally non-passable over CapTP—only the promise behavior crosses boundariescancelledgetter is allowed, no other own propertiesScaling Considerations
None.
Documentation Considerations
Provides README and DESIGN.
Testing Considerations
@endo/cancelCompatibility Considerations
AbortControllercancelledcontinue to work unchangedUpgrade Considerations
Packages using the
makePromiseKitpattern for cancellation may optionally migrate tomakeCancelKitfor improved ergonomics and the synchronous.cancelledgetter.