Skip to content

ISSUE-46: fix notification SSE buffering#527

Open
dongmucat wants to merge 1 commit into
mainfrom
agent/backend/0fb0ae56
Open

ISSUE-46: fix notification SSE buffering#527
dongmucat wants to merge 1 commit into
mainfrom
agent/backend/0fb0ae56

Conversation

@dongmucat

Copy link
Copy Markdown
Collaborator

Summary

  • Prevent notification SSE requests from passing through ContentCachingResponseWrapper in RequestLoggingFilter.
  • Set text/event-stream, Cache-Control: no-cache, no-transform, and X-Accel-Buffering: no for the notification SSE response.
  • Add regression coverage proving SSE uses the raw response while normal API requests keep cached logging behavior.

Backend evidence

  • The notification manager already registers emitters and sends connected/notification events.
  • Reproduction test failed before the fix because /api/web/notifications/sse reached the filter chain as a ContentCachingResponseWrapper, which can buffer streaming bytes.
  • The same test passes after bypassing the wrapper for notification SSE.

Tests

  • cd server && JDK_JAVA_OPTIONS="-XX:+EnableDynamicAgentLoading" ./mvnw -pl skillhub-app -am test -Dtest=RequestLoggingFilterTest -Dsurefire.failIfNoSpecifiedTests=false
  • make test-backend-app

Fixes #519

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.

[Bug] sse connected,but can not reveive notification by sse connection.

1 participant