Preserve the FastAPI request for disconnect polling

For any execution that produces no output for KEEPALIVE_INTERVAL (for example time.sleep(10)), the local request has already been rebound above to the JSON websocket payload string, so this call passes a str into _wait_for_result. On the first timeout _wait_for_result calls await request.is_disconnected(), which raises AttributeError and aborts the stream instead of continuing or interrupting on an actual disconnect. Keep the FastAPI Request in a separate variable from the Jupyter execute payload.

Useful? React with 👍 / 👎.

Write disconnect not handled

Medium Severity

The streaming disconnect handler only catches CancelledError, GeneratorExit, and internal _ClientDisconnected. On Starlette ≥ 1.0 a dead client can also surface as connection errors (including Starlette’s ClientDisconnect) while flushing a yielded chunk, so those paths skip interrupt() even though the finally block drops the execution entry.

 

Reviewed by Cursor Bugbot for commit cf6255b. Configure here.

🔴 Critical: The new request: Optional[Request] = None parameter on execute() is shadowed by the local assignment at line 415 — request = self._get_execute_request(message_id, complete_code, False) — which returns a JSON str. When self._wait_for_result(message_id, request=request) then runs, request.is_disconnected() on the string raises AttributeError on the first 5s keepalive tick. AttributeError isn't in execute()'s except (CancelledError, GeneratorExit, _ClientDisconnected) tuple, so it propagates, the kernel is never interrupted, and the very test_subsequent_execution_works_after_client_timeout test this PR exists to fix will still fail. Fix: rename the local payload variable (e.g. exec_payload).

What the bug is

The PR adds a new request: Optional[Request] = None parameter to ContextWebSocket.execute() (messaging.py:341) so that _wait_for_result can poll request.is_disconnected() and raise _ClientDisconnected to interrupt the kernel. The problem is that inside execute()'s retry loop at lines 411-415, the local variable name request is reused for the JSON payload to send over the WebSocket:

request = self._get_execute_request(message_id, complete_code, False)
await self._ws.send(request)

_get_execute_request is annotated -> str and returns json.dumps(...). This rebinds the local name request to a string, shadowing the Request parameter for the remainder of the function.

How it manifests

At line 421, async for item in self._wait_for_result(message_id, request=request): then passes the JSON string (not the original Starlette Request) into _wait_for_result. Inside _wait_for_result (lines 295-296):

if request is not None and await request.is_disconnected():
    raise _ClientDisconnected()

A non-empty string is truthy, so Python evaluates request.is_disconnected — strings have no such attribute, so Python raises AttributeError: 'str' object has no attribute 'is_disconnected' before await even runs.

Why existing code doesn't prevent it

execute()'s exception handler only catches (asyncio.CancelledError, GeneratorExit, _ClientDisconnected) (line 423). AttributeError is not in that tuple, so it escapes the generator and propagates up through StreamingListJsonResponse, breaking the streaming response entirely. The kernel-interrupt path on line 427 never runs.

Impact

The KEEPALIVE_INTERVAL is 5 seconds, so any execution that takes longer than 5 seconds will trigger the first timeout and immediately crash with AttributeError. This means:

1. The disconnect-detection mechanism this PR adds is dead on arrival — _ClientDisconnected is never raised in normal flow.
2. The keepalive mechanism itself is now broken for any execution > 5s.
3. The kernel is never interrupted on client disconnect, leaving the failing-test scenario (test_subsequent_execution_works_after_client_timeout, which sleeps 300s) unfixed.

Step-by-step proof

1. Client POST /execute with code that runs long enough to require keepalives (e.g. time.sleep(300)).
2. post_execute (main.py:126) calls ws.execute(..., request=request), where request is the Starlette Request.
3. Inside execute(), the for-loop body executes request = self._get_execute_request(message_id, complete_code, False) at line 415. request is now '{"header": {...}, ...}' (a JSON string).
4. await self._ws.send(request) at line 416 sends that string, then break exits the loop with request still bound to the string.
5. Line 421 calls self._wait_for_result(message_id, request=request) — passing the string.
6. After 5 seconds with no kernel output, asyncio.wait_for(queue.get(), timeout=5) raises TimeoutError.
7. The handler evaluates if request is not None and await request.is_disconnected():. The string is non-None, so it accesses request.is_disconnected → AttributeError.
8. The AttributeError propagates out of _wait_for_result into execute()'s try block.
9. The except (CancelledError, GeneratorExit, _ClientDisconnected) clause does not match, so it propagates out of execute(), breaking the streaming response. self.interrupt() is never called.
10. Result: client gets a broken stream, kernel keeps running, and the next POST /execute blocks waiting for the lock (the same regression asyncio.Lock in messaging.py not released on client disconnect → cascading timeouts #213 was meant to fix).

How to fix

Rename the local payload variable so the Request parameter is preserved. Smallest patch is at lines 411-416:

exec_payload = self._get_execute_request(message_id, complete_code, False)
await self._ws.send(exec_payload)

(_cleanup_env_vars and change_current_directory also use a local named request for the JSON payload, but they don't take a Request parameter, so the shadowing only matters in execute().)