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

feat(graphql): Make gql query timeout optional #1473

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

josephelliot-wk
Copy link

Problem

The latest version of the graphql package enforces a timeout on its queries.

Solution

Make the request timeout duration optional, and if omitted, will not invoke timeout on the query request stream.

Why

We prefer to let our underlying http library (Dio) handle our timeouts, and as such decouple the responsibility from the gql client itself.

Not only does this let us define timeout logic in one place (the Dio config) and share it with non-gql requests, but it also vastly simplifies our unit testing story. We only mock our Dio client (and not the gql client itself), which effectively lets us build a testing story as close to production as possible. I couldn't for the life of me figure out how to adapt our unit tests to accommodate .timeout injected within the gql client itself. (The unit test failures were basically "a timer still exists" even though the requests have already finished.)

The workaround before this PR

Because our unit testing uses the gql client itself (instead of mocking it), bumping to the latest beta version of this package started failing basically every unit test.

See: #1457

To get around this, we basically hijacked the pub cache to force a custom patch that removes .timeout entirely. We can remove this patch if this PR is merged.

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.

1 participant