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

Is there a convenient way to use graphql-request with an AbortController? #182

Closed
martaver opened this issue Jul 24, 2020 · 9 comments
Closed

Comments

@martaver
Copy link

Is there a convenient way to use graphql-request with an AbortController?

I would like to debounce and cancel in-flight requests when they are repeated.

@zolbooo
Copy link

zolbooo commented Aug 4, 2020

You can pass options to the GraphQLClient constructor:

import { GraphQLClient, gql } from 'graphql-request'

async function main() {
  const endpoint = 'https://api.graph.cool/simple/v1/cixos23120m0n0173veiiwrjr'
  const controller = new AbortController()

  const graphQLClient = new GraphQLClient(endpoint, {
    signal: controller.signal,
  })

  const query = gql`
    {
      Movie(title: "Inception") {
        releaseDate
        actors {
          name
        }
      }
    }
  `

  return graphQLClient.request(query);
}

const result = main().catch((error) => console.error(error));
controller.abort();

@andycmaj
Copy link

andycmaj commented Oct 6, 2020

i actually have some thoughts on this. we're doing this ourselves, and ran into a few really annoying issues. we patched a workaround but i think there could be an API change to graphql-request to make this less hacky...

the problem

(also see mysticatea/abort-controller#25)

when you create GraphQLClient, you pass an instance of RequestInit to the ctor, and this instance is re-used for every internal fetch invocation that GraphQLClient makes.

if you create a brand new GraphQLClient on each request, you won't really see an issue, but if you use a client that is singleton/static for some scope, you will hit this issue:

once an AbortSignal triggers, there is absolutely no way to reset it from outside the AbortSignal module. The fact that it's triggered is module-static w/r/t the signal instance itself, and there isn't a way to access/reset.

so once it's triggered, every subsequent fetch that uses this same instance of RequestInit (which now contains a permanently aborted signal) will fail and will look like it's immediately timed out.

my hacky patch

I've created a fork of abort-controller package that lets me reset an AbortSignal instance.

I then defined a TimeoutSignal to consume and reset this patched AbortController:

import AbortController from 'botany-abort-controller';

const signalMap = new WeakMap();

export const getTimeoutSignal = timeout => {
  if (!Number.isInteger(timeout)) {
    throw new TypeError(`Expected an integer, got ${typeof timeout}`);
  }

  const controller = new AbortController();

  const timeoutId = setTimeout(() => {
    controller.abort();
  }, timeout);

  signalMap.set(controller.signal, timeoutId);

  return controller;
};

export const resetTimeout = (controller, timeout) => {
  const signal = controller.signal;
  controller.reset(signal);
  clearTimeout(signalMap.get(signal));
  const timeoutId = setTimeout(() => {
    controller.abort();
  }, timeout);

  signalMap.set(controller.signal, timeoutId);
};

here's what usage looks like:

// graphql client
const client = (
  timeout: AbortController,
  headers: Record<string, string> = {}
) =>
  new GraphQLClient(process.env.GRAPHQL_URL, {
    signal: timeout.signal,
  });

// polly-js policy
const withRetries: (
  controller: AbortController
) => SdkFunctionWrapper = controller => <T>(
  action: () => Promise<T>,
  operationName: string
) =>
  polly()
    .handle((err: Error) => {
      if (
        /AbortError/.exec(err.name) ||
        err.message.includes('connect ETIMEDOUT') ||
        err.message.includes('network timeout') ||
        err.message.includes('write EPIPE')
      ) {
        warn('GraphqlClient:NetworkError', err, { operationName });
        return true;
      }

      error('GraphqlClient:NonTimeoutError', err, { operationName });
      return false;
    })
    .waitAndRetry([200, 400, 800])
    .executeForPromise(info => {
      if (info.count === 3) {
        error('GraphqlClient:MaxRetries', {
          ...info,
          operationName,
        });
      }

/* HERE IS WHERE YOU RESET THE ABORTED SIGNAL ========= */
      // reset timeout signal BEFORE executing the call. otherwise, timer
      // will be started before the `waitAndRetry` backoff times
      resetTimeout(controller, TIMEOUT_MS);
/* HERE IS WHERE YOU RESET THE ABORTED SIGNAL ========= */

      return withClientTiming(action, operationName);
    });

// getting a client
const getGraphqlClient = () => {
  const timeout = getTimeoutSignal(TIMEOUT_MS);

  return getSdk(client(timeout), withRetries(timeout));
};

less hacky solutions

  1. just submit my patch as a change to AbortController (i'm planning on doing this when i have time).
  2. change the API of GraphqlClient a bit to allow passing RequestInit in on every request, not just when you construct the client.

@nicolasburtey
Copy link

  1. would be nice!

@ba55ie
Copy link

ba55ie commented Dec 15, 2020

Yes, I agree with option 2. 👍

Before switching to graphql-request I used ky for my GraphQL needs. That library is a simple wrapper around fetch and has support for timeouts and retries.

There's also ky-universal for isomorphic apps. Switching to ky-universal could be beneficial to this project without adding too much bloat. And it should fix some issues like #229 and #192.

@nicolasburtey
Copy link

an out of the box retry/timeout mechanism is for me to be the main thing that is lacking compared to the apollo-client library

@bombillazo
Copy link

Please, can we pass fetch options per request, not just on client creation???

@dhirjgupta
Copy link

Just wondering if there was any news / updates on this. Just found myself running into the same problem.

@arnaudbzn
Copy link
Contributor

Just push this PR #303 to support a signal per request 🤞

@jasonkuhrt
Copy link
Owner

jasonkuhrt commented Jan 8, 2022

Closed by #303 Thanks @arnaudbzn !

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

No branches or pull requests

9 participants