🐛 bug: remove SSE Next and clarify SSE handler docs#4247
Conversation
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/dc926626-f1b7-4a9d-bbef-6499f6a82125 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/e15ef545-45b5-4ca4-8f1a-0cf1bcbe0198 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4247 +/- ##
==========================================
- Coverage 91.23% 91.23% -0.01%
==========================================
Files 126 126
Lines 12341 12339 -2
==========================================
- Hits 11259 11257 -2
Misses 678 678
Partials 404 404
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/1c5638bb-b13f-4be8-ab83-fc6bec272964 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/1c5638bb-b13f-4be8-ab83-fc6bec272964 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/1c5638bb-b13f-4be8-ab83-fc6bec272964 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/1c5638bb-b13f-4be8-ab83-fc6bec272964 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/1c5638bb-b13f-4be8-ab83-fc6bec272964 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: f362b3a | Previous: 2b45a43 | Ratio |
|---|---|---|---|
Benchmark_Compress/Zstd (github.com/gofiber/fiber/v3/middleware/compress) - B/op |
1 B/op |
0 B/op |
+∞ |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Pull request overview
This PR updates the SSE middleware package to behave and be described as a terminal SSE route handler, removing the Next/skip hook and improving the disconnect test to more reliably simulate client interruption via fiber.TestConfig timeouts.
Changes:
- Removed
Config.Nextand thec.Next()skip path fromsse.New(). - Updated SSE docs to remove the
Nextoption from the config table/default config. - Replaced the
Next-skipping test with a timeout-driven interrupted-client test that asserts the stream observes disconnect andOnCloseis invoked with an error.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| middleware/sse/sse_test.go | Reworks the disconnect test to use fiber.TestConfig timeouts and validate Done()/Err()/OnClose behavior. |
| middleware/sse/sse.go | Removes Next handling and updates package/API wording toward “handler”. |
| middleware/sse/config.go | Removes Next from the public config surface and default config. |
| docs/middleware/sse.md | Updates documented config surface/default config to match removal of Next. |
Comments suppressed due to low confidence (2)
middleware/sse/sse.go:6
- The package-level docs still say "core middleware" (line 6), but this PR renames the package and New() docs to use "handler" terminology. Please update the remaining references in this comment block so the package description is internally consistent (e.g., change "core middleware" to "core handler" or similar).
// The package focuses on the SSE transport: response headers, wire formatting,
// flushing, heartbeat comments, and disconnect detection via flush errors.
// Application-specific concerns such as topics, replay storage, authentication,
// and pub/sub fan-out intentionally stay outside the core middleware.
middleware/sse/config.go:18
- Removing the exported
Config.Nextfield is a breaking API change for users configuring SSE. Please add an explicit migration note (e.g., in the release notes / whats_new) stating that SSE no longer supports Next/skip semantics and must be mounted as a route handler (or use the dedicated skip middleware upstream).
type Config struct {
// Handler writes events to the stream.
//
// Required.
Handler Handler
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/51a2082a-3d5b-4fd0-9cd9-5c8042c08280 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
…c-42ea-a892-cec7cfab0cc3
Description
This PR removes the SSE
Nextskip hook so the package behaves as a terminal SSE route handler, updates the interrupted-client test to usefiber.TestConfig, and clarifies the SSE documentation to match the current API.The changes keep the SSE package focused on stream transport behavior, make the disconnect test use Fiber’s supported test path, and document that
Config.Handleris required becausesse.New()panics when it is omitted ornil.Changes introduced
/docs/middleware/sse.mdto use handler terminology consistently and to document thatConfig.Handleris required andsse.New()panics when it isnil.Nextskip hook from the public SSE config and clarified the requiredHandlercontract to make the API behavior more explicit and less error-prone.Handlerusage, and the disconnect coverage now usesfiber.TestConfig{FailOnTimeout: false, Timeout: time.Second}inmiddleware/sse/sse_test.go.Type of change
Please delete options that are not relevant.
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md