Skip to content

fix: avoid AlreadySentError on duplicate SSE GET#20

Merged
dbernheisel merged 1 commit intodbernheisel:mainfrom
efleming:fix/sse-already-sent-on-duplicate-get
Apr 28, 2026
Merged

fix: avoid AlreadySentError on duplicate SSE GET#20
dbernheisel merged 1 commit intodbernheisel:mainfrom
efleming:fix/sse-already-sent-on-duplicate-get

Conversation

@efleming
Copy link
Copy Markdown
Contributor

Fixes #19.

Summary

  • When a second GET /mcp arrives for a session that already has a
    tracked SSE stream, maybe_track_session_stream/1 correctly responds
    with HTTP 409 and a JSON-RPC -32000 error and halts the conn.
  • However, the dispatch(%Plug.Conn{method: \"GET\"}, opts) clause
    was unconditionally calling put_resp_header / send_chunked on
    the returned conn, which raised Plug.Conn.AlreadySentError.
  • Branch on conn.halted after maybe_track_session_stream/1 so the
    409 response is returned as-is and SSE setup is skipped.

Test plan

  • New regression test in test/phantom/plug_test.exs opens two
    consecutive GETs with the same session id and asserts the
    second returns status == 409 with code: -32000 and
    \"Only one SSE stream is allowed per session\".
  • Test fails on main with Plug.Conn.AlreadySentError raised
    at lib/phantom/plug.ex:285.
  • Test passes with this change.
  • Full suite: mix test — 201 tests, 0 failures (1 pre-existing
    skip), Elixir 1.19.1 / OTP 28.1.

Notes for downstream users

While this lands, downstream apps can wrap Phantom.Plug and rescue
Plug.Conn.AlreadySentError to fall back to a 409 JSON-RPC error
themselves. The wrapper is no longer necessary once this is released.

When a second `GET /mcp` arrives for an existing session,
`maybe_track_session_stream/1` correctly responds with HTTP 409 and a
JSON-RPC `-32000` error. However, the conn was already sent and halted at
that point, so the surrounding `dispatch/2` clause then tried to call
`put_resp_header` and `send_chunked` on the same conn, raising
`Plug.Conn.AlreadySentError`.

Branch on `conn.halted` after `maybe_track_session_stream/1` so the 409
response is returned as-is and SSE setup is skipped.

Adds a regression test that performs two consecutive GETs on the same
session id and asserts the second returns a clean 409 with the expected
JSON-RPC error envelope.
Copy link
Copy Markdown
Owner

@dbernheisel dbernheisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much!

@dbernheisel dbernheisel merged commit f4d86fe into dbernheisel:main Apr 28, 2026
3 checks passed
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.

Plug.Conn.AlreadySentError when client opens a second SSE GET on the same session

2 participants