Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [Bug]: There is a conflict between app.NewCtxFunc and app.Use #3319

Open
3 tasks done
attains-cloud opened this issue Feb 22, 2025 · 5 comments · May be fixed by #3328
Open
3 tasks done

🐛 [Bug]: There is a conflict between app.NewCtxFunc and app.Use #3319

attains-cloud opened this issue Feb 22, 2025 · 5 comments · May be fixed by #3328

Comments

@attains-cloud
Copy link

attains-cloud commented Feb 22, 2025

Bug Description

CustomCtx no effect when app.newCtxFunc and app.use simultaneously existing

How to Reproduce

Steps to reproduce the behavior:

  1. create custom ctx
      type Ctx struct {
      	fiber.DefaultCtx
      }
      
      func (c *Ctx) JSON(data any, ctype ...string) error {
      	return c.DefaultCtx.JSON(Response{
      		Code:    http.StatusOK,
      		Data:    data,
      		Message: http.StatusText(http.StatusOK),
      	}, ctype...)
      }

      type Response struct {
      	Code    int    `json:"code,omitempty" xml:"code,omitempty" yaml:"code,omitempty"`
      	Data    any    `json:"data,omitempty" xml:"data,omitempty" yaml:"data,omitempty"`
      	Message string `json:"message,omitempty" xml:"message,omitempty" yaml:"message,omitempty"`
      }
  1. create app with newCtxFunc:
              app := fiber.New(fiber.Config{
			JSONEncoder: sonic.Marshal,
			JSONDecoder: sonic.Unmarshal,
		})

		app.NewCtxFunc(func(app *fiber.App) fiber.CustomCtx {
			return &Ctx{
				DefaultCtx: *fiber.NewDefaultCtx(app),
			}
		})

                app.Get("/vvv", func(ctx fiber.Ctx) error {
			return ctx.JSON(fiber.Map{"a": "b"})
		})
                _ = app.Listen(":8080")
  1. request:
curl localhost:8080/vvv   
{"code":200,"data":{"a":"b"},"message":"OK"}%
  1. create app with newCtxFunc and app.use:
          app := fiber.New(fiber.Config{
			JSONEncoder: sonic.Marshal,
			JSONDecoder: sonic.Unmarshal,
		})

		app.NewCtxFunc(func(app *fiber.App) fiber.CustomCtx {
			return &Ctx{
				DefaultCtx: *fiber.NewDefaultCtx(app),
			}
		})

                app.Use(func(c fiber.Ctx) error {
			return c.Next()
		})

                app.Get("/vvv", func(ctx fiber.Ctx) error {
			return ctx.JSON(fiber.Map{"a": "b"})
		})
                _ = app.Listen(":8080")
  1. request:
curl localhost:8080/vvv
{"a":"b"}

Expected Behavior

app.NewCtxFunc and app.Use can coexist

Fiber Version

v3.0.0-beta.4

Code Snippet (optional)

package main

import "github.com/gofiber/fiber/v3"
import "log"

func main() {
  app := fiber.New()

  // Steps to reproduce

  log.Fatal(app.Listen(":3000"))
}

Checklist:

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

welcome bot commented Feb 22, 2025

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

@JIeJaitt
Copy link
Contributor

This is because when not using app.Use, HTTP -> customRequestHandler -> nextCustom (which executes the route handler directly);

If you use middleware, testCustomCtx doesn't implement the c.Next() method, so it will call *DefaultCtx's c.Next() method by default, and the c.Next() method will call nextCustom again internally and then go to call the route handler.

But here is the problem, because *DefaultCtx's Next method receiver is *DefaultCtx , he can't handle the JSON method implemented by CustomCtx, so your custom middleware fails. @ReneWerner87 @gaby @efectn Am I right?

// Next executes the next method in the stack that matches the current route.
func (c *DefaultCtx) Next() error {
	// Increment handler index
	c.indexHandler++

	// Did we execute all route handlers?
	if c.indexHandler < len(c.route.Handlers) {
		// Continue route stack
		return c.route.Handlers[c.indexHandler](c)
	}

	// Continue handler stack
	if c.app.newCtxFunc != nil {
		_, err := c.app.nextCustom(c)
		return err
	}

	_, err := c.app.next(c)
	return err
}

If you want to achieve the effect you want, maybe you need to customize a Next method belonging to testCustomCtx. I didn't do any specific experiments, you can try it or I will when I have time. @attains-cloud

@JIeJaitt
Copy link
Contributor

If u want to access the full CustomCtx in inherited methods, we have to override those methods like Next(). The following unit test verified my suspicions:

type Response struct {
	Code    int    `json:"code"`
	Data    any    `json:"data"`
	Message string `json:"message"`
}

type testCustomCtx struct {
	*DefaultCtx
}

func (c *testCustomCtx) JSON(data any, ctype ...string) error {
	return c.DefaultCtx.JSON(Response{
		Code:    StatusOK,
		Data:    data,
		Message: utils.StatusMessage(StatusOK),
	}, ctype...)
}

func (c *testCustomCtx) Next() error {
	// Increment handler index
	c.indexHandler++

	// Did we execute all route handlers?
	if c.indexHandler < len(c.route.Handlers) {
		// Continue route stack
		return c.route.Handlers[c.indexHandler](c)
	}

	// Continue handler stack
	_, err := c.app.nextCustom(c)
	return err
}

func Test_App_CustomCtx_With_Use(t *testing.T) {
	t.Parallel()

	t.Run("without middleware", func(t *testing.T) {
		app := New()
		app.NewCtxFunc(func(app *App) CustomCtx {
			return &testCustomCtx{
				DefaultCtx: NewDefaultCtx(app),
			}
		})

		app.Get("/test", func(c Ctx) error {
			return c.JSON(Map{"a": "b"})
		})

		resp, err := app.Test(httptest.NewRequest(MethodGet, "/test", nil))
		require.NoError(t, err)
		require.Equal(t, 200, resp.StatusCode)

		body, err := io.ReadAll(resp.Body)
		require.NoError(t, err)

		expected := `{"code":200,"data":{"a":"b"},"message":"OK"}`
		require.Equal(t, expected, string(body))
	})

	t.Run("with middleware", func(t *testing.T) {
		app := New()
		app.NewCtxFunc(func(app *App) CustomCtx {
			return &testCustomCtx{
				DefaultCtx: NewDefaultCtx(app),
			}
		})

		app.Use(func(c Ctx) error {
			t.Logf("Before Next() - context type: %T", c)
			err := c.Next()
			t.Logf("After Next() - context type: %T", c)
			return err
		})

		app.Get("/test", func(c Ctx) error {
			if _, ok := c.(*testCustomCtx); !ok {
				t.Logf("Handler received context of type: %T", c)
			}
			return c.JSON(Map{"a": "b"})
		})

		resp, err := app.Test(httptest.NewRequest(MethodGet, "/test", nil))
		require.NoError(t, err)
		require.Equal(t, 200, resp.StatusCode)

		body, err := io.ReadAll(resp.Body)
		require.NoError(t, err)

		expected := `{"code":200,"data":{"a":"b"},"message":"OK"}`
		require.Equal(t, expected, string(body),
			"Custom context JSON format is lost when using middleware")
	})
}

When we customize the Next method of testCustomCtx, the unit test called with middleware will pass, otherwise it will report an error
Image

Image

@JIeJaitt
Copy link
Contributor

JIeJaitt commented Feb 25, 2025

@ReneWerner87 @gaby @efectn As mentioned above, this issue may not be considered a bug, and perhaps we should add a hint to the documentation telling users how to use both app.NewCtxFunc and app.Use.

@ReneWerner87
Copy link
Member

just as a hint
in this pull request which is still being implemented
https://github.com/gofiber/fiber/pull/3296/files
we try to do without these extra processes for the custom ctx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants