Skip to content

fix: keep SSE alive past idle timeout (Safari "Connection lost")#376

Merged
tomasz-tomczyk merged 1 commit intomainfrom
safari-sse-fix
Apr 28, 2026
Merged

fix: keep SSE alive past idle timeout (Safari "Connection lost")#376
tomasz-tomczyk merged 1 commit intomainfrom
safari-sse-fix

Conversation

@tomasz-tomczyk
Copy link
Copy Markdown
Owner

Summary

  • Send an initial :\n\n SSE comment so EventSource.onopen fires immediately (Safari delays this until the first body byte).
  • Add a 30s heartbeat ticker so the stream isn't closed by the server's 60s IdleTimeout.

Why

Safari was surfacing repeated "Could not connect to the server" errors on the review page. Root cause was idle SSE connections hitting the server's IdleTimeout: 60s — Chromium and Firefox auto-reconnect more silently, which is why this looked Safari-specific even though the root cause affected all browsers.

Review

  • Code review: passed
  • go-expert agent assessed perf cost: negligible (~0.1 syscalls/sec per connection, no allocations, no extra goroutine).

Test plan

  • New unit test TestEvents_SafariCompat asserts the initial :\n\n frame and heartbeat (with sseHeartbeatInterval shrunk to 50ms).
  • gofmt -l . clean
  • golangci-lint run ./... clean
  • go test -race -count=1 ./... passes (46s)
  • Manual verification in Safari via Docker container: stream stayed connected past 60s, no console errors, crit comment propagated via SSE.

🤖 Generated with Claude Code

Send an initial `:\n\n` SSE comment so EventSource fires onopen immediately
(Safari delays this until the first body byte), and add a 30s heartbeat
ticker so the stream isn't closed by the server's 60s IdleTimeout.

Without the heartbeat, idle SSE connections hit the IdleTimeout and Safari
surfaces repeated "Could not connect to the server" errors. Chromium and
Firefox auto-reconnect more silently, which is why this looked Safari-
specific even though the root cause affected all browsers.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.59%. Comparing base (331350c) to head (fb7732f).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #376      +/-   ##
==========================================
+ Coverage   66.55%   66.59%   +0.04%     
==========================================
  Files          19       19              
  Lines        8169     8176       +7     
==========================================
+ Hits         5437     5445       +8     
+ Misses       2310     2309       -1     
  Partials      422      422              
Flag Coverage Δ
e2e 33.81% <57.14%> (+0.01%) ⬆️
unit 62.61% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tomasz-tomczyk tomasz-tomczyk merged commit bf75eba into main Apr 28, 2026
5 of 6 checks passed
@tomasz-tomczyk tomasz-tomczyk deleted the safari-sse-fix branch April 28, 2026 16:19
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.

1 participant