Skip to content
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

AbortSignal integration #46

Closed
rektide opened this issue Jul 2, 2021 · 8 comments
Closed

AbortSignal integration #46

rektide opened this issue Jul 2, 2021 · 8 comments
Assignees

Comments

@rektide
Copy link

rektide commented Jul 2, 2021

Hello,

This library looks wonderful. One thing that makes it a bit hard for us & probably other folks to adopt & use is that it escew's the web (and increasingly the rest of js too) standard AbortSignal cancellation protocols.

There are some various api differences between the standard JS abort protocols & the CancellationToken presently in use in cockatiel. The main difference is that CancellationToken is both the means to observe change, and the system for doing the change. This is reminisce of the old jQuery Deferrable, which was a precursor to promises, where the Deferrable was both the promise & the resolver. In JS today, usually the thing that does the actuating/changing is separate from the system for observing. That is reflected in AbortSignal and it's pair AbortController, where AbortSignal is the output (somewhat alike a sychronously readable Promise), and AbortController can be used to actuate the signal (quite strongly resembling the obsolete JS Deferred, which contrary to jquery, divided the promise & the resolver ala AbortSignal/AbortController). First used in Fetch, today, AbortSignal is being widely integrated into the JS ecosystem.

Please pardon my long-windedness above. I find it interesting to provide some historical roots for the topic, as they show how things have moved over time, but I am mainly writing this ticket to talk about today & the future & how cockatiel can be the best JS citizen it can be. CancellationToken has a pretty nice, convenient, & very powerful set of APIs atop it, and I generally commend the Microsoft teams that have shaped it over the years. However, I feel like it's in cockatiel's best interest prefer to integrate with the JS language ecosystem, if possible, and feel like we should try to switch to AbortSignal/AbortController as the primary means to handle/deal-with cancellation concerns.

@connor4312
Copy link
Owner

connor4312 commented Jul 21, 2021

Yea, I want to move to AbortSignal in a v3 of this library. I've no specific timeline for that, although I would welcome PR's.

@connor4312 connor4312 self-assigned this Sep 29, 2021
@HonzaMac
Copy link

HonzaMac commented Nov 8, 2021

I tried to use this pattern to forward new AbortController().signal into CancellationToken.
Could this work?

import fetch, {RequestInfo, RequestInit, Response} from 'node-fetch';

export const fetchRetry = (url: RequestInfo, init: RequestInit): Promise<Response> => {
  const cts = new CancellationTokenSource();
  if (init?.signal) {
    init.signal.addEventListener('abort', () => cts.cancel(), {once: true});
  }

  return retryPolicy.execute(() => fetch(url, init), cts.token);
};

@connor4312
Copy link
Owner

I've now done this. However, as AbortController/AbortToken is only available in Node.js starting in v16, I will wait to publish this until the end of April when Node 12 enters EOL and Node 18 is released.

@HonzaMac
Copy link

Can we use some of available Abort Controller polyfills? And replace it with native on April?

@connor4312
Copy link
Owner

I published 3.0.0-beta.0. It's a prerelease tag so will need to be explicitly installed.

@HonzaMac
Copy link

Thank you! Will try it on mine project.

@dev-usa
Copy link

dev-usa commented Jan 5, 2022

I published 3.0.0-beta.0. It's a prerelease tag so will need to be explicitly installed.

Trying to use cockatiel in a node application that I am working on. Attempted to use both the current version and the 3.0.0-beta.0 based on this thread and received the AbortController is not defined error in both cases. Please see below.

Tried using the AbortController polyfill but could not get it to work.

Any suggestions?

I am running node v14.17.3

2022-01-05T19:28:56.943Z        undefined       ERROR   Uncaught Exception      {"errorType":"ReferenceError","errorMessage":"AbortController is not defined","stack":["ReferenceError: AbortController is not defined","   
 at Object.<anonymous> (/var/task/node_modules/cockatiel/dist/common/abort.js:7:34)","    
at Module._compile (internal/modules/cjs/loader.js:1085:14)","    
at Object..js (internal/modules/cjs/loader.js:1114:10)","    
at Module.load (internal/modules/cjs/loader.js:950:32)","    
at Function._load (internal/modules/cjs/loader.js:790:14)","    
at Module.require (internal/modules/cjs/loader.js:974:19)","    
at require (internal/modules/cjs/helpers.js:92:18)","    
at Object.<anonymous> (/var/task/node_modules/cockatiel/dist/CircuitBreakerPolicy.js:4:17)","    
at Module._compile (internal/modules/cjs/loader.js:1085:14)","    
at Object..js (internal/modules/cjs/loader.js:1114:10)"]}
time="2022-01-05T19:28:56.95" level=panic msg="ReplyStream not available"

@connor4312
Copy link
Owner

connor4312 commented Jan 5, 2022

Yes, you must use it in an environment where AbortController is available, that's why it's a prerelease for now. Or define the AbortController in some way; I don't know anything about that polyfill or your use of it.

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

No branches or pull requests

4 participants