fix(core): Block every reexecute of an in-flight operation

[EDjur]

Summary

client.reexecuteOperation blocks a reexecute of an operation that is still in flight (#3573, fixing #3565), but the blocking else branch also clears that operation's dispatched flag. The next reexecute during the same request therefore passes nextOperation's deduplication check, and a second network request goes out. In effect every second reexecute during one flight gets through.

We hit this with Graphcache, which reexecutes an active query on every dependency change. Under a stream of subscription events, every second event started another copy of a slow list query, so one page held several concurrent copies of the same request.

Repro against @urql/core alone — one slow exchange, no cache exchange — counting the operations that reach the exchange while a single cache-first query is in flight. Verified against a build of current main:

scenario	main	this PR
1 / 2 / 3 / 4 reexecutes during one flight	1 / 2 / 2 / 3	1 / 1 / 1 / 1
stale first result, then a reexecute (#3254 shape)	1 → 2	1 → 2
one reexecute during a flight (#3565 shape)	1	1
reexecute after the result, subscriber still active	2	2
network-only reexecute during a flight	1	1

The last four rows are the behaviours unblocks stale operations and blocks reexecuting operations that are in-flight already cover, and both stay green.

Set of changes

packages/core/src/client.ts — the flag only has to be cleared when the reexecute replaced an operation already waiting in the queue, so that the queued operation is not deduplicated as the queue drains. An in-flight operation keeps its flag.

} else {
  if (queued) dispatched.delete(operation.key);
  Promise.resolve().then(dispatchOperation);
}

packages/core/src/client.test.ts — new test blocks repeated reexecutes of operations that are in-flight, placed after blocks reexecuting operations that are in-flight: three reexecuteOperation calls while the first request is pending must not dispatch again. It fails on main with 2 dispatches and passes with the change.

Plus a patch changeset for @urql/core. Only @urql/core is affected; pnpm test, pnpm run check and pnpm run lint are green across the monorepo.

Why the flag was cleared, and why the stale path covers it

The clearing was introduced by #3363 to unblock an operation stalled after an optimistic mutation (#3254). Since then that case is handled by the stale-result path in onPush — result.stale && !result.hasNext is "an optimistic mutation or a partial result" and clears the key — which is what the existing unblocks stale operations test exercises. That leaves the queue case as the only reason to clear the flag in reexecuteOperation, and the queued check keeps it.

One question for you

The unconditional clear was also a hedge for an operation that an exchange swallows without emitting any result — for example Graphcache dropping a cache-first miss that is blocked by an optimistic update, or one whose key is in its reexecutingOperations guard. With this change such an operation stays blocked until its teardown, whereas before it would have been let through on the next reexecute.

I could not construct that shape without the stale emission that onPush already handles, but you know the exchanges far better than I do. If it is reachable, this needs a release valve rather than the plain queued check, and I am happy to rework it.

[changeset-bot]

🦋 Changeset detected

Latest commit: 1187bd5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@urql/core	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[EDjur]

Hey maintainers. As you can see from the description I indeed used Claude for this PR. But happy to discuss without involving Claude too.

It's a very small fix for something we've ran into internally, where a burst of web socket events kept invalidating a slow query and we'd end up with several copies of it in flight at once instead of one.

[EDjur]

Friendly ping @JoviDeCroock

[JoviDeCroock]

I did a small research but this reintroduces an old bug. I have little time atm so I can elaborate further next week, you can have claude figure it out by saying 'look at the historical diff of your changes, they relate to graphcache loop bugs'

[EDjur]

Hah, thank you! :)

That's useful context.

No rush on this btw I have a workaround deployed already on our side of things.

[EDjur]

I actually couldn't find how this re-introduces the old bug so parking this for a bit.

And again, no rush but you did get me curious now...

[JoviDeCroock]

Had a look, this brings back #3254.

• fix(core): Force client.reexecuteOperation to dispatch #3363 made reexecuteOperation clear dispatched so a reexecute always goes through. That fixed Graphcache stalling a query after an optimistic mutation caused a cache miss.
• fix(client): eager unblocking of in-flight operations #3573 blocked reexecutes of in-flight operations but kept the clear in the else. The "every second reexecute gets through" behaviour is that leftover escape hatch from fix(core): Force client.reexecuteOperation to dispatch #3363.
• This PR removes it.

The stale path in onPush only helps when the query already has a result. Graphcache can drop a dispatched query without emitting anything: cacheMissOps$ filters out cache-first misses that are blocked by an optimistic update or are in reexecutingOperations. So yes, the case you asked about is reachable.

Repro with a real client + Graphcache: { authors { id name } } resolves → an optimistic mutation invalidates Author:123 (response pending) → a new { authors { id } } mounts and gets dropped as a blocked miss → let the network resolve. On main the second query recovers when the first query's refetch reexecutes it again. With this PR it never gets a result until it unmounts, even though the data is in the cache.

CI doesn't catch it because the Graphcache test from #3363 mocks reexecuteOperation, and #3573 switched the core test from #3363 to a mutation. Both tests below pass on main and fail with this PR.

core: client.test.ts (deduplication behavior)

// See https://github.com/urql-graphql/urql/issues/3254
it('unblocks query operations dropped by an exchange on reexecuteOperation', async () => {
  const onOperation = vi.fn();
  const onResult = vi.fn();

  let hasSent = false;
  const exchange: Exchange = () => ops$ =>
    pipe(
      ops$,
      filter(op => op.kind !== 'teardown'),
      onPush(onOperation),
      map(op => ({
        hasNext: false,
        stale: false,
        data: 'test',
        operation: op,
      })),
      // Drop the first result, like Graphcache does for a blocked cache miss
      filter(() => hasSent || !(hasSent = true))
    );

  const client = createClient({
    url: 'test',
    exchanges: [exchange],
  });

  const operation = makeOperation('query', queryOperation, {
    ...queryOperation.context,
    requestPolicy: 'cache-first',
  });

  pipe(client.executeRequestOperation(operation), subscribe(onResult));
  expect(onOperation).toHaveBeenCalledTimes(1);
  expect(onResult).toHaveBeenCalledTimes(0);

  client.reexecuteOperation(operation);
  await Promise.resolve();
  client.reexecuteOperation(operation);
  await Promise.resolve();

  expect(onOperation).toHaveBeenCalledTimes(2);
  expect(onResult).toHaveBeenCalledTimes(1);
});

graphcache: cacheExchange.test.ts (optimistic updates, needs Exchange, fromPromise and subscribe imported)

// See https://github.com/urql-graphql/urql/issues/3254
it('does not stall queries dropped while blocked by an optimistic update', async () => {
  const authorsQuery = gql`
    query {
      authors {
        id
        name
      }
    }
  `;

  const authorIdsQuery = gql`
    query {
      authors {
        id
      }
    }
  `;

  const mutation = gql`
    mutation {
      deleteAuthor {
        id
        name
      }
    }
  `;

  const author = { __typename: 'Author', id: '123', name: 'Author' };

  let pending: (() => void)[] = [];
  const tick = async () => {
    for (let i = 0; i < 20; i++) await Promise.resolve();
  };
  const flush = async () => {
    await tick();
    const resolvers = pending;
    pending = [];
    resolvers.forEach(resolve => resolve());
    await tick();
  };

  // Network exchange that holds responses until `flush()`
  const network: Exchange = () => ops$ =>
    pipe(
      ops$,
      filter(op => op.kind !== 'teardown'),
      mergeMap(op =>
        fromPromise(
          new Promise<OperationResult>(resolve => {
            pending.push(() =>
              resolve({
                operation: op,
                data:
                  op.kind === 'mutation'
                    ? { __typename: 'Mutation', deleteAuthor: author }
                    : { __typename: 'Query', authors: [author] },
                hasNext: false,
                stale: false,
              })
            );
          })
        )
      )
    );

  const client = createClient({
    url: 'http://0.0.0.0',
    exchanges: [
      cacheExchange({
        optimistic: {
          deleteAuthor: () => ({ ...author, name: '[REDACTED OFFLINE]' }),
        },
        updates: {
          Mutation: {
            deleteAuthor: (_data, _args, cache) => {
              cache.invalidate({ __typename: 'Author', id: '123' });
            },
          },
        },
      }),
      network,
    ],
  });

  const onResult = vi.fn();

  pipe(client.query(authorsQuery, {}), subscribe(() => {}));
  await flush();

  // The optimistic update invalidates Author:123 while the mutation is in-flight
  pipe(client.mutation(mutation, {}), subscribe(() => {}));
  await tick();

  // A new query mounts, misses, and is dropped since it's blocked by the optimistic update
  pipe(client.query(authorIdsQuery, {}), subscribe(onResult));
  await tick();
  expect(onResult).toHaveBeenCalledTimes(0);

  for (let i = 0; i < 3; i++) await flush();
  expect(onResult).toHaveBeenCalled();
});

Deduping the in-flight burst makes sense, but I think Graphcache has to tell the client when it drops a miss, rather than us removing the escape hatch.

[EDjur]

Thanks for that write-up and evidence.

But it sounds like we'll need to make changes both here and in the graphcache-exchange to solve this then? I admit I'm a little out of my depth here in my urql knowledge and I'd rather not just slop Claude at the problem now that it's not so simple anymore.

Wdyt? Worth solving? For me personally I already have a workaround deployed so I'm in no rush.