🐛 [Bug]: Cannot set header to 'text/event-stream' with ctx.Set

[michealroberts]

Bug Description

As above, I had a middleware running on version 2.31.0 of Go finer, where I would utilise the following:

import "github.com/gofiber/fiber/v2"

func SetServerSentEventsHeaders(c *fiber.Ctx) error {
    c.Set("Content-Type", "text/event-stream")
    c.Set("Cache-Control", "no-cache")
    c.Set("Connection", "keep-alive")
    c.Set("Transfer-Encoding", "chunked")
    return c.Next()
}

My associated test suite was passing on version 2.31.0 but on version 2.48.0 it seems to be failing, instead, the Content-Type header is still set to 'text/plain'

How to Reproduce

As above.

Expected Behavior

It should be able to set the Content-Type header to "text/event-stream".

Fiber Version

2.48.0

Code Snippet (optional)

package sse

import "github.com/gofiber/fiber/v2"

func SetServerSentEventsHeaders(c *fiber.Ctx) error {
  c.Set("Content-Type", "text/event-stream")
  c.Set("Cache-Control", "no-cache")
  c.Set("Connection", "keep-alive")
  c.Set("Transfer-Encoding", "chunked")
  return c.Next()
}

func New() fiber.Handler {
  return func(c *fiber.Ctx) error {
    return SetServerSentEventsHeaders(c)
  }
}

Associated test (failing):

package sse

import (
    "testing"

    "github.com/gofiber/fiber/v2"
    "github.com/gofiber/utils"
    "github.com/valyala/fasthttp"
)

func TestSSEDefaults(t *testing.T) {
  app := fiber.New(fiber.Config{
    IdleTimeout: -1,
    ReadTimeout: 0,
  })

  app.Use(New())

  t.Helper()

  h := app.Handler()

  // Test default GET response headers
  ctx := &fasthttp.RequestCtx{}
  ctx.Request.Header.SetMethod(fiber.MethodGet)
  h(ctx)

  utils.AssertEqual(t, "text/event-stream", string(ctx.Response.Header.Peek(fiber.HeaderContentType)))
  utils.AssertEqual(t, "no-cache", string(ctx.Response.Header.Peek(fiber.HeaderCacheControl)))
  utils.AssertEqual(t, "keep-alive", string(ctx.Response.Header.Peek(fiber.HeaderConnection)))

  ctx.Response.Header.Set(fiber.HeaderConnection, "close")
  ctx.Response.ConnectionClose()

  app.Shutdown()
}

Checklist:

• [X] I agree to follow Fiber's Code of Conduct.
• [X] I have checked for existing issues that describe my problem prior to opening this one.
• [X] I understand that improperly formatted bug reports may be closed without explanation.

[welcome[bot]]

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[ReneWerner87]

@michealroberts reason is this https://github.com/gofiber/fiber/pull/1847

since the handler is a middleware and contains only one next you must have at least one handler with logic

func TestSSEDefaults(t *testing.T) {
    app := fiber.New(fiber.Config{
        IdleTimeout: -1,
        ReadTimeout: 0,
    })

    app.Use(New())
    // needed
    app.Get("/", func(ctx *fiber.Ctx) error {
        return nil
    })

    t.Helper()

    h := app.Handler()

    // Test default GET response headers
    ctx := &fasthttp.RequestCtx{}
    ctx.Request.Header.SetMethod(fiber.MethodGet)
    h(ctx)

    utils.AssertEqual(t, "text/event-stream", string(ctx.Response.Header.Peek(fiber.HeaderContentType)))
    utils.AssertEqual(t, "no-cache", string(ctx.Response.Header.Peek(fiber.HeaderCacheControl)))
    utils.AssertEqual(t, "keep-alive", string(ctx.Response.Header.Peek(fiber.HeaderConnection)))

    ctx.Response.Header.Set(fiber.HeaderConnection, "close")
    ctx.Response.ConnectionClose()

    app.Shutdown()
}

[michealroberts]

@ReneWerner87 Ok the reason seems logical, but worth noting that this (i.e., my tests) were working on version 2.31.0 of Fiber ...

[ReneWerner87]

@michealroberts

reason is this https://github.com/gofiber/fiber/pull/1847

yes as I said before it is because of the feature that came in the next release

https://github.com/gofiber/fiber/releases/tag/v2.32.0

[michealroberts]

@ReneWerner87 Totally understand. So you wouldn't consider that a breaking change?

[ReneWerner87]

yes is a kind of breaking change

for me this was more a bug, because a middleware without a real handler which processes the content and only forwards it to other middlewares is rather wrong

this only affects those who had been using it incorrectly, so for 95% of people it is not a breaking change

[michealroberts]

yes is a kind of breaking change

for me this was more a bug, because a middleware without a real handler which processes the content and only forwards it to other middlewares is rather wrong

Yeh I also appreciate this. It's kind of a bug, but introduces a change that will break a lot of previous code. Could you update the release notes of 2.32.0 with a breaking change note please, this should really have been a v3 release RFC imho, but as it borders the line between bug and feature, it's a though one to partition.

[ReneWerner87]

yes I can do

[ReneWerner87]