Skip to content

GH-5858: Add session ID logging in WebFluxMcpSessionTransport error handler#5859

Open
ArpanC6 wants to merge 2 commits intospring-projects:mainfrom
ArpanC6:GH-5858
Open

GH-5858: Add session ID logging in WebFluxMcpSessionTransport error handler#5859
ArpanC6 wants to merge 2 commits intospring-projects:mainfrom
ArpanC6:GH-5858

Conversation

@ArpanC6
Copy link
Copy Markdown

@ArpanC6 ArpanC6 commented Apr 22, 2026

What

Added session ID logging in the doOnError handler of
WebFluxMcpSessionTransport and removed the TODO comment.

Why

The doOnError handler was logging errors without any session identifier,
making it impossible to trace which session caused the error in production
with multiple concurrent sessions.

A TODO comment existed: // TODO log with sessionid

How

  • Added a @Nullable String sessionId field to WebFluxMcpSessionTransport
  • Added a setSessionId() method
  • Called setSessionId() in handleSseConnection() after session creation
  • Updated doOnError to log the session ID with the error message

Closes #5858

ArpanC6 added 2 commits April 20, 2026 11:22
…first one

Signed-off-by: Arpan Chakraborty <chakrabortyarpan151@gmail.com>
…ansport error handler

Signed-off-by: Arpan Chakraborty <chakrabortyarpan151@gmail.com>
@ArpanC6
Copy link
Copy Markdown
Author

ArpanC6 commented Apr 22, 2026

Note: The first commit (2a9d948) is from a previous fix (GH-5832)
already included in PR #5833. Only the second commit (d4f2fc3)
is relevant to this PR.

Copy link
Copy Markdown

@anuragg-saxenaa anuragg-saxenaa left a comment

Choose a reason for hiding this comment

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

LGTM. Clean, targeted fix — WebFluxMcpSessionTransport now stores its sessionId for better error logging. The null check with sessionId in the catch block is exactly the right approach for MCP server debugging.

@ArpanC6
Copy link
Copy Markdown
Author

ArpanC6 commented Apr 22, 2026

LGTM. Clean, targeted fix — WebFluxMcpSessionTransport now stores its sessionId for better error logging. The null check with sessionId in the catch block is exactly the right approach for MCP server debugging.

Thank you for the review.

@redinside-dev
Copy link
Copy Markdown

LGTM. Clean, targeted fix — WebFluxMcpSessionTransport now stores its sessionId for better error logging in production. The setSessionId() call placement in handleSseConnection() is correct and non-invasive. Ready to merge.

Copy link
Copy Markdown

@anuragg-saxenaa anuragg-saxenaa left a comment

Choose a reason for hiding this comment

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

LGTM — session ID injection in doOnError handler is clean. The setSessionId() call after session creation is the correct placement. Changes are minimal and targeted (+17/-3 lines). Fixes the TODO comment and enables production debugging. Ready to merge.

Copy link
Copy Markdown

@anuragg-saxenaa anuragg-saxenaa left a comment

Choose a reason for hiding this comment

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

LGTM — session ID injection in doOnError handler is clean. setSessionId call after session creation is correct. Minimal and targeted +17/-3. Fixes TODO and enables production debugging. Ready to merge.

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.

Missing session ID in error log of WebFluxMcpSessionTransport

3 participants