Skip to content

Commit 91e50cf

Browse files
fix(query_manager): guard late stream events after request timeout
Guard the response stream's onData handler the same way onError is guarded, and cancel the subscription once the completer settles. Stream.timeout forwards a TimeoutException without cancelling the source, so a successful response arriving after the timeout reached an unguarded completer.complete on an already-completed completer, throwing "Bad state: Future already completed" from an async callback that escaped the surrounding try/catch and crashed the app. Also complete with an error on an empty stream so the future cannot hang, restoring the prior responseStream.first semantics. Fixes #1525 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 822a903 commit 91e50cf

2 files changed

Lines changed: 90 additions & 8 deletions

File tree

packages/graphql/lib/src/core/query_manager.dart

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,42 @@ class QueryManager {
281281
responseStream = responseStream.timeout(timeout);
282282
}
283283

284-
// Listen for the first response or error
285-
responseStream.listen(completer.complete,
286-
onError: (Object error, StackTrace stackTrace) {
287-
if (!completer.isCompleted) {
288-
// We return the first error encountered
289-
completer.completeError(error, stackTrace);
290-
}
291-
});
284+
// Listen for the first response or error.
285+
//
286+
// Both `onData` and `onError` must be guarded against completing the
287+
// completer more than once. `Stream.timeout` forwards a
288+
// `TimeoutException` to `onError` without cancelling the source
289+
// subscription, so a late event (e.g. a successful response that
290+
// arrives after the timeout has already settled the completer) can
291+
// still be delivered. Completing an already-completed completer throws
292+
// "Bad state: Future already completed" from inside this async callback,
293+
// which escapes the surrounding try/catch and crashes the app.
294+
//
295+
// Once the completer is settled we cancel the subscription so the
296+
// underlying request does not keep running after we have a result.
297+
late final StreamSubscription<Response> subscription;
298+
subscription = responseStream.listen(
299+
(response) {
300+
if (!completer.isCompleted) {
301+
completer.complete(response);
302+
}
303+
subscription.cancel();
304+
},
305+
onError: (Object error, StackTrace stackTrace) {
306+
if (!completer.isCompleted) {
307+
// We return the first error encountered
308+
completer.completeError(error, stackTrace);
309+
}
310+
subscription.cancel();
311+
},
312+
onDone: () {
313+
// The stream closed without emitting; surface this the same way
314+
// `Stream.first` used to so the awaiting future does not hang.
315+
if (!completer.isCompleted) {
316+
completer.completeError(StateError('No element'));
317+
}
318+
},
319+
);
292320

293321
// Await the response or error
294322
response = await completer.future;

packages/graphql/test/query_manager_test.dart

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:async';
2+
13
import 'package:gql/language.dart';
24
import 'package:graphql/client.dart';
35
import 'package:mockito/mockito.dart';
@@ -42,5 +44,57 @@ void main() {
4244
);
4345
client.queryManager.refetchQuery<dynamic>(observable.queryId);
4446
});
47+
48+
// Regression test for https://github.com/zino-hofmann/graphql-flutter/issues/1525
49+
//
50+
// When a request times out and the underlying link later delivers a
51+
// successful response, the late event must be ignored rather than
52+
// completing the already-settled completer (which used to throw an
53+
// uncaught "Bad state: Future already completed").
54+
test(
55+
"late response after queryRequestTimeout does not crash",
56+
() async {
57+
final controller = StreamController<Response>();
58+
when(
59+
link.request(any),
60+
).thenAnswer((_) => controller.stream);
61+
62+
final timeoutClient = GraphQLClient(
63+
cache: getTestCache(),
64+
link: link,
65+
queryRequestTimeout: const Duration(milliseconds: 50),
66+
);
67+
68+
// The request never emits before the timeout fires.
69+
final result = await timeoutClient.query(
70+
QueryOptions<dynamic>(
71+
document: parseString("{ fetchPerson { name } }"),
72+
),
73+
);
74+
75+
expect(result.hasException, isTrue);
76+
expect(result.exception!.linkException, isA<UnknownException>());
77+
expect(
78+
(result.exception!.linkException as UnknownException)
79+
.originalException,
80+
isA<TimeoutException>(),
81+
);
82+
83+
// The real response arrives after the timeout already settled the
84+
// request. This must not throw an uncaught error.
85+
controller.add(
86+
Response(
87+
data: <String, dynamic>{
88+
'fetchPerson': <String, dynamic>{'name': 'late'},
89+
},
90+
response: <String, dynamic>{},
91+
),
92+
);
93+
await controller.close();
94+
95+
// Pump the event loop so any late delivery would surface here.
96+
await Future<void>.delayed(const Duration(milliseconds: 20));
97+
},
98+
);
4599
});
46100
}

0 commit comments

Comments
 (0)