Skip to content

fix SSE GET panic on HTTP-only backends #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
dkhokhlov wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix SSE GET panic on HTTP-only backends #102

dkhokhlov wants to merge 4 commits into from

Conversation

@dkhokhlov
Copy link

@dkhokhlov dkhokhlov commented Dec 7, 2025 •
edited
Loading

Summary

  • Add an --http-streaming-only (HTTP_STREAMING_ONLY) flag/env to the CLI and documentation so deployments can signal that their backend only supports POST-based streaming.
  • Propagate the flag through mcpproxy.Run and ProxyRouter, rejecting SSE GET requests with a 405 before traffic reaches the backend, which stops the net/http: abort Handler panic.
  • Expand pkg/proxy tests to cover the SSE rejection path and confirm the backend isn’t touched in streaming-only mode.

Type of Change

  • fix: A bug fix
  • test: Adding missing tests or correcting existing tests
  • feat
  • docs
  • style
  • refactor
  • perf
  • build
  • ci
  • chore
  • revert

Related Issues

Fixes the SSE GET panic seen when proxying an HTTP-only backend (#103).

Copilot AI review requested due to automatic review settings December 7, 2025 23:01
Copilot AI reviewed Dec 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a panic that occurs when HTTP-only backends receive SSE GET requests by adding a configuration flag to reject such requests before they reach the backend.

Key changes:

  • Added --http-streaming-only CLI flag and HTTP_STREAMING_ONLY environment variable to configure SSE rejection
  • Modified proxy routing to detect and reject SSE GET requests with HTTP 405 when streaming-only mode is enabled
  • Added test coverage to verify SSE requests are rejected and backends are not called in streaming-only mode

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
main.go Added httpStreamingOnly boolean flag with CLI and environment variable support, passed to the Run function
pkg/mcp-proxy/main.go Updated Run function signature to accept httpStreamingOnly parameter and pass it to ProxyRouter
pkg/proxy/proxy.go Added httpStreamingOnly field to ProxyRouter struct, implemented SSE detection logic, and added rejection before proxying
pkg/proxy/proxy_test.go Updated existing test to pass new httpStreamingOnly parameter and added new test to verify SSE rejection behavior
docs/docs/configuration.md Added documentation for the new --http-streaming-only flag in the Proxy Options table

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dkhokhlov dkhokhlov force-pushed the pr_Fix_SSE_GET_panic branch from cdbc961 to 04e18c2 Compare December 7, 2025 23:18
@dkhokhlov
Implemented Copilot’s feedback:
432136e 
 - isSSEGetRequest now strips media type parameters before matching, so headers like text/event-stream; charset=utf-8 or with q-values are handled.
 - Expanded TestProxyRouter_HTTPStreamingOnlyRejectsSSE to cover parameterized Accept headers, multiple values, q-values, and to ensure POST + Accept: text/event-stream still passes through to the backend.
@dkhokhlov dkhokhlov requested a review from Copilot December 7, 2025 23:27
Copilot AI reviewed Dec 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dkhokhlov dkhokhlov requested a review from Copilot December 7, 2025 23:37
Copilot AI reviewed Dec 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Dec 10, 2025

Codecov Report

❌ Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
main.go 0.00% 3 Missing ⚠️
pkg/mcp-proxy/main.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@dkhokhlov dkhokhlov requested a review from Copilot December 10, 2025 15:26
Copilot AI reviewed Dec 10, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dkhokhlov