Skip to content

fix(runtime-sdk): fix contextvars not being shared in WSGI streaming response - #288

Open
ryanking13 wants to merge 2 commits into
mainfrom
gyeongjae/flask-stream
Open

ryanking13 wants to merge 2 commits into
mainfrom
gyeongjae/flask-stream

Conversation

@ryanking13

Copy link
Copy Markdown
Contributor

fix #287

WSGI streaming response use stack switching. However, they should use the same contextvars, since they are in the same request context.

@ask-bonk

ask-bonk Bot commented Oct 7, 2026

Copy link
Copy Markdown

LGTM

github run

@clanky-the-clanker

Copy link
Copy Markdown

✅ Approved — The fix works: one copied context now covers calling the app, iterating it and closing it. The new tests check that values are shared within a stream, kept separate between concurrent streams, and reset when a stream is cancelled. No issues found.

The decision is advisory: this comment never approves, blocks or requests changes on the pull request.


✅ All checks passed

🔒 Security: ✅ LGTM

I found no security concerns.


📖 AGENTS.md: ✅ LGTM

This change does not need any AGENTS.md updates.


📋 Code Quality: ✅ LGTM

I found no issues in this change.

  • Context handling in wsgi.py: process_request now runs the app, iter(result), every next() and close() through one copied contextvars.Context. Priming the first chunk and the later pulls therefore share one context, and closing the iterable can reset ContextVar tokens in the context that created them. Exceptions from the iterator still propagate through context.run.
  • Cleanup order: the try/finally in close_all closes wsgi.input even if closing the iterable raises.
  • Test coverage: the new tests cover the fix. The workerd WSGI tests check that a ContextVar set by the app is visible during streaming, that concurrent streams stay separate, and that cancelling a stream resets the token. The Flask tests exercise stream_with_context itself.
  • Test dependency: the pytest-asyncio<1.2.0 pin in flask-tests/pyproject.toml matches the other test packages in the repo.

⚡ Performance: ✅ LGTM

I found no performance concerns. Running each chunk through context.run adds very little overhead.


📝 Documentation: ✅ LGTM

I found no documentation issues.


Reviewed commit: 00df2794

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.

runtime-sdk: Flask stream_with_context fails across WSGI streaming callback contexts

1 participant