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

Timeout configuration #103

Closed
fungiboletus opened this issue Sep 4, 2018 · 9 comments · Fixed by #173
Closed

Timeout configuration #103

fungiboletus opened this issue Sep 4, 2018 · 9 comments · Fixed by #173

Comments

@fungiboletus
Copy link

Hei, would it be possible to support configuring the timeout ?

@bfelbo
Copy link

bfelbo commented Feb 23, 2019

Any updates on this? It would be great to have.

@ninnemannk
Copy link

I found after a bit of digging that the graphql-request library delegates additional options to the underlying node-fetch module. So, instantiating a client like so, delegates the timeout to the node-fetch library.

  const client = new GraphQLClient(config.url, {
    timeout: 30000,
    headers: {
      Authorization: `Bearer ${token}`
    }
  })

Delegation: https://github.com/prisma/graphql-request/blob/master/src/index.ts#L29
node-fetch options: https://www.npmjs.com/package/node-fetch#options


It would be fantastic to get this added to the documentation.

@prakhar241
Copy link

My IDE complains about using timeout being not defined -
https://github.com/prisma-labs/graphql-request/blob/master/src/types.ts#L7?

image

How to make it work?

@keithlayne
Copy link

@prakhar241 TS won't let you add excess properties when you pass an object literal. The solution is to construct the options as a separate const and then pass them in:

const options = { timeout: 30000, mode: 'cors' };
const graphQLClient = new GraphQLClient(requestURL, options);

@jescalan
Copy link

node-fetch has deprecated the timeout option in their latest release in favor of passing an AbortSignal. Unfortunately, passing this at the top level doesn't work correctly, as each request must have its own signal, and if the request was successful, the timeout on the signal must be cleared.

Unfortunately, this means we need to find another way to add a timeout option. I'd be happy to put up a PR to add it to this library if that's cool with the maintainers!

@jasonkuhrt
Copy link
Owner

jasonkuhrt commented Aug 28, 2020

@jescalan I'm not familiar with AbortSignal yet or how it could impact the graphql-request API. Could you start a new issue to show and discuss the design further? Once we have a some better understanding around the implications then I can better tell you if the PR is safe to start or not (safe meaning: will the work be wasted).

@naruaway
Copy link

naruaway commented Jun 23, 2021

For people who found this issue to look for how to implement timeout for graphql-request:

Since graphql-request supports custom fetch like the above, we can easily add global timeout option without changing code of graphql-request with the following helper:

const createFetchWithTimeout = (timeout: number) => async (
    input: RequestInfo,
    init?: RequestInit
) => {
    if (init.signal) {
        throw new Error(
            "it looks like graphql-request started using AbortSignal on its own. Please check graphql-request's recent updates"
        );
    }

    const controller = new AbortController();

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

    try {
        return await fetch(input, { ...init, signal: controller.signal });
    } finally {
        clearTimeout(timerId);
    }
};

Then we can use it like the following:

const client = new GraphQLClient(YOUR_API_ENDPOINT, {
    // 2000ms timeout
    fetch: createFetchWithTimeout(2000),
});

@yhou2021
Copy link

yhou2021 commented May 8, 2023

If your project is using TypeScript, the solution above will need to add URL as an alternative type of input, like this:

const createFetchWithTimeout =
  (timeout?: number) =>
  async (input: RequestInfo | URL, init?: RequestInit)

Otherwise TS compiler would complain that RestquestInfo is not assignable to RequestInfo | URL.

@dmitry-zaporozhan
Copy link

Shorter version:

fetch(input, { ...init, signal: AbortSignal.timeout(timeout) })

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 a pull request may close this issue.

10 participants