Commit f7c8353
oliver
fix: don't overclaim cause in error message, test the _in_flight fix, cite precedent
Addresses review feedback on the session-not-initialized error fix:
- The error message asserted a specific cause ("this session_id's
stream was reconnected or lost") that _initialization_state has no
way to actually verify -- a session that was never initialized at
all (e.g. a client bug) hits the same branch and would get told a
story that's simply false in that case. Reworded to state the fact
(an initialize handshake is required) without asserting why this
particular session never completed one.
- Cited the existing INVALID_REQUEST precedent in
streamable_http_manager.py's "Session not found" responses (same
class of session-validity condition) in the inline comment, rather
than relying on JSON-RPC semantics alone.
- This fix has an untested, unmentioned side effect: answering via
responder.respond() means RequestResponder.__exit__ now sees
_completed=True and fires on_complete, popping the request out of
_in_flight. The old bare RuntimeError bypassed the responder's
context-manager cleanup entirely, so every rejected pre-init request
leaked an _in_flight entry for the life of the session -- a client
hammering an uninitialized session could grow it unboundedly. Added
an assertion pinning this (len(session._in_flight) == 0 after the
rejected request), so a future refactor can't silently reintroduce
the leak.
- Also assert on error.data ("session_not_initialized"), the
machine-readable discriminator this fix introduces -- message text
isn't meant to be programmatically parsed, this is.
Verified: tests/server/test_session.py (7/7) and the broader
tests/server/ + tests/shared/test_session.py suite (521 passed, same
pre-existing unrelated websockets-extra collection error as before)
both pass. ruff check / ruff format --check / pyright clean on both
changed files.1 parent 8058392 commit f7c8353
2 files changed
Lines changed: 33 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
211 | 211 | | |
212 | 212 | | |
213 | 213 | | |
214 | | - | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
215 | 224 | | |
216 | 225 | | |
217 | 226 | | |
218 | 227 | | |
219 | 228 | | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
224 | 235 | | |
225 | 236 | | |
226 | 237 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
473 | 473 | | |
474 | 474 | | |
475 | 475 | | |
| 476 | + | |
| 477 | + | |
476 | 478 | | |
477 | 479 | | |
478 | 480 | | |
| |||
483 | 485 | | |
484 | 486 | | |
485 | 487 | | |
486 | | - | |
| 488 | + | |
| 489 | + | |
487 | 490 | | |
488 | 491 | | |
489 | 492 | | |
490 | 493 | | |
491 | 494 | | |
492 | | - | |
| 495 | + | |
493 | 496 | | |
494 | 497 | | |
495 | 498 | | |
| |||
510 | 513 | | |
511 | 514 | | |
512 | 515 | | |
| 516 | + | |
513 | 517 | | |
514 | 518 | | |
515 | 519 | | |
| |||
530 | 534 | | |
531 | 535 | | |
532 | 536 | | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
0 commit comments