Commit 5342ca4
authored
fix: Task state is not persisted to task store after client disconnect (#472)
# Issue
It's been described well in #464 :
> When the client-side connection is terminated, the EventConsumer stops
processing. As a result, any changes to the task state after the
disconnection are not persisted to the TaskStore. The task itself
continues running in the background, but its updated state is no longer
reflected in the TaskStore.
This has been addressed in this PR by simply adding a catch for
`(asyncio.CancelledError, GeneratorExit)` in the
`on_message_send_stream` method.
However, adding that revealed a difference in semantics between Python
3.13+ and <3.13 for `EventQueue.close()`. I have also addressed that.
# How it's reproduced
[@azyobuzin](https://github.com/azyobuzin) provided a detailed guide on
this in #464 . My only addition would be to add loggers for
`a2a.server.events.event_queue` and `a2a.server.events.event_consumer`
to get a better understanding of what's happening under the hood.
# Fix
## Code
- Ensure streaming continues persisting events after client disconnect
via background consumption by adding a catch for
`(asyncio.CancelledError, GeneratorExit)` in the
`on_message_send_stream` method.
- Align EventQueue.close() behavior on Python ≥3.13 and ≤3.12 (graceful
vs. immediate).
## Tests
### Event queue tests (`tests/server/events/test_event_queue.py`)
Added/updated tests to verify:
- Graceful close on ≥3.13 waits for drain and children.
- Immediate close clears queues and propagates.
- To support Python 3.10, when simulating ≥3.13 using sys.version_info,
inject a dummy queue.shutdown on asyncio.Queue so tests don’t fail on
runtimes without it. I've seen this pattern used in existing tests too.
### Request handler tests
(`tests/server/request_handlers/test_default_request_handler.py`)
Added `test_disconnect_persists_final_task_to_store` which tests the
flow described in issue #464 :
- Starts streaming, yields first event, then simulates client
disconnect.
- Background consumer persists the final Task to `InMemoryTaskStore`.
- Uses `wait_until` to await disappearance of the specific
`background_consume:{task_id}` task, then asserts `TaskState.completed`.
### General
Added a cleanup for lingering background tasks, I think it's an
improvement for my earlier PR where I've tracked background tasks.
# Misc
Ruff `0.13.0` now fails the check for unused variables, made some
minimal changes as suggested by the linter, irrelevant to the issue:
underscores for `_payload` and `_task_manager`.
Fixes #4641 parent 5ec0788 commit 5342ca4
File tree
5 files changed
+183
-36
lines changed- src/a2a
- client/transports
- server
- events
- request_handlers
- tests/server
- events
- request_handlers
5 files changed
+183
-36
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
206 | 206 | | |
207 | 207 | | |
208 | 208 | | |
209 | | - | |
| 209 | + | |
210 | 210 | | |
211 | 211 | | |
212 | 212 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
138 | | - | |
139 | | - | |
140 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
141 | 150 | | |
142 | 151 | | |
143 | 152 | | |
| |||
152 | 161 | | |
153 | 162 | | |
154 | 163 | | |
155 | | - | |
| 164 | + | |
156 | 165 | | |
157 | | - | |
158 | | - | |
159 | | - | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
160 | 178 | | |
161 | 179 | | |
162 | 180 | | |
163 | 181 | | |
164 | 182 | | |
165 | 183 | | |
166 | 184 | | |
167 | | - | |
168 | | - | |
169 | | - | |
| 185 | + | |
| 186 | + | |
170 | 187 | | |
171 | | - | |
172 | 188 | | |
173 | 189 | | |
174 | 190 | | |
| |||
Lines changed: 12 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
314 | 314 | | |
315 | 315 | | |
316 | 316 | | |
317 | | - | |
| 317 | + | |
318 | 318 | | |
319 | 319 | | |
320 | 320 | | |
| |||
379 | 379 | | |
380 | 380 | | |
381 | 381 | | |
382 | | - | |
| 382 | + | |
383 | 383 | | |
384 | 384 | | |
385 | 385 | | |
386 | 386 | | |
387 | 387 | | |
| 388 | + | |
| 389 | + | |
388 | 390 | | |
389 | 391 | | |
390 | | - | |
391 | | - | |
392 | 392 | | |
393 | 393 | | |
394 | 394 | | |
| |||
397 | 397 | | |
398 | 398 | | |
399 | 399 | | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
400 | 408 | | |
401 | 409 | | |
402 | 410 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
271 | 271 | | |
272 | 272 | | |
273 | 273 | | |
274 | | - | |
275 | | - | |
276 | | - | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | 274 | | |
281 | | - | |
282 | | - | |
283 | 275 | | |
284 | 276 | | |
285 | 277 | | |
| |||
290 | 282 | | |
291 | 283 | | |
292 | 284 | | |
293 | | - | |
294 | | - | |
295 | | - | |
| 285 | + | |
296 | 286 | | |
297 | 287 | | |
298 | 288 | | |
299 | 289 | | |
300 | 290 | | |
301 | 291 | | |
302 | 292 | | |
303 | | - | |
304 | | - | |
305 | | - | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
306 | 296 | | |
| 297 | + | |
| 298 | + | |
307 | 299 | | |
308 | | - | |
309 | 300 | | |
310 | | - | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
311 | 326 | | |
312 | 327 | | |
313 | 328 | | |
| |||
345 | 360 | | |
346 | 361 | | |
347 | 362 | | |
348 | | - | |
349 | | - | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
350 | 368 | | |
351 | 369 | | |
352 | | - | |
| 370 | + | |
353 | 371 | | |
354 | 372 | | |
355 | 373 | | |
356 | | - | |
| 374 | + | |
357 | 375 | | |
358 | 376 | | |
359 | 377 | | |
| |||
Lines changed: 105 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
| 2 | + | |
2 | 3 | | |
3 | 4 | | |
4 | 5 | | |
| |||
48 | 49 | | |
49 | 50 | | |
50 | 51 | | |
| 52 | + | |
51 | 53 | | |
52 | 54 | | |
53 | 55 | | |
| |||
1331 | 1333 | | |
1332 | 1334 | | |
1333 | 1335 | | |
| 1336 | + | |
| 1337 | + | |
| 1338 | + | |
| 1339 | + | |
| 1340 | + | |
| 1341 | + | |
| 1342 | + | |
| 1343 | + | |
| 1344 | + | |
1334 | 1345 | | |
1335 | 1346 | | |
1336 | 1347 | | |
| |||
1367 | 1378 | | |
1368 | 1379 | | |
1369 | 1380 | | |
| 1381 | + | |
| 1382 | + | |
| 1383 | + | |
1370 | 1384 | | |
1371 | 1385 | | |
1372 | 1386 | | |
| |||
1385 | 1399 | | |
1386 | 1400 | | |
1387 | 1401 | | |
| 1402 | + | |
| 1403 | + | |
| 1404 | + | |
| 1405 | + | |
| 1406 | + | |
| 1407 | + | |
| 1408 | + | |
| 1409 | + | |
| 1410 | + | |
| 1411 | + | |
| 1412 | + | |
| 1413 | + | |
| 1414 | + | |
| 1415 | + | |
| 1416 | + | |
| 1417 | + | |
| 1418 | + | |
| 1419 | + | |
| 1420 | + | |
| 1421 | + | |
| 1422 | + | |
| 1423 | + | |
| 1424 | + | |
| 1425 | + | |
| 1426 | + | |
| 1427 | + | |
| 1428 | + | |
| 1429 | + | |
| 1430 | + | |
| 1431 | + | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
| 1435 | + | |
| 1436 | + | |
| 1437 | + | |
| 1438 | + | |
| 1439 | + | |
| 1440 | + | |
| 1441 | + | |
| 1442 | + | |
| 1443 | + | |
| 1444 | + | |
| 1445 | + | |
| 1446 | + | |
| 1447 | + | |
| 1448 | + | |
| 1449 | + | |
| 1450 | + | |
| 1451 | + | |
| 1452 | + | |
| 1453 | + | |
| 1454 | + | |
| 1455 | + | |
| 1456 | + | |
| 1457 | + | |
| 1458 | + | |
| 1459 | + | |
| 1460 | + | |
| 1461 | + | |
| 1462 | + | |
| 1463 | + | |
| 1464 | + | |
| 1465 | + | |
| 1466 | + | |
| 1467 | + | |
| 1468 | + | |
| 1469 | + | |
| 1470 | + | |
| 1471 | + | |
| 1472 | + | |
| 1473 | + | |
| 1474 | + | |
| 1475 | + | |
| 1476 | + | |
| 1477 | + | |
| 1478 | + | |
| 1479 | + | |
| 1480 | + | |
| 1481 | + | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
| 1486 | + | |
1388 | 1487 | | |
1389 | 1488 | | |
1390 | 1489 | | |
| |||
1505 | 1604 | | |
1506 | 1605 | | |
1507 | 1606 | | |
| 1607 | + | |
| 1608 | + | |
| 1609 | + | |
| 1610 | + | |
| 1611 | + | |
| 1612 | + | |
1508 | 1613 | | |
1509 | 1614 | | |
1510 | 1615 | | |
| |||
0 commit comments