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]: incorrect selection of the error handler if one of the sub apps is mounted on "/" #3003

Open
3 tasks done
DaryaHom opened this issue May 16, 2024 · 2 comments

Comments

@DaryaHom
Copy link

Bug Description

I create two sub apps with different error handlers and mount one of them on the β€œ/” path and the other on the β€œ/two” path.

If an error occurs in handler, the (app *App)ErrorHandler method can incorrectly define the error handler for the second sub app. You can see this in the code snippet.

This happens because the paths β€œ/” and β€œ/two” return the same number of parts (2) from strings.Split(prefix, β€œ/” ) , and the path β€œ/two” is prefixed with β€œ/”. For the method to work correctly, the path β€œ/” must be defined as only 1 part:

func (app *App) ErrorHandler(ctx *Ctx, err error) error {
	...
	for prefix, subApp := range app.mountFields.appList {
		if prefix != "" && strings.HasPrefix(ctx.path, prefix) {
                        if prefix == "/" {
                                parts = 1
                        } else {
			parts := len(strings.Split(prefix, "/"))
			}
	...

How to Reproduce

Steps to reproduce the behavior:

  1. Run the code snippet (may take a few times).
  2. Check the output in the stdout.

Expected Behavior

The error handler for the sub app must be defined correctly.
In my example, only the twoErrorHandler must be used for the sub app two.

Fiber Version

v2.52.0

Code Snippet (optional)

package main

import (
	"errors"
	"fmt"
	"log"
	"net/http"

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

// Custom error handlers.
func oneErrorHandler(c *fiber.Ctx, err error) error {
	// Custom error handling logic here.
	fmt.Println("ONE error handler")

	// Return from handler
	return fiber.DefaultErrorHandler(c, err)
}

func twoErrorHandler(c *fiber.Ctx, err error) error {
	// Custom error handling logic here.
	fmt.Println("TWO error handler")

	// Return from handler
	return fiber.DefaultErrorHandler(c, err)
}

func handler(c *fiber.Ctx) error {
	// Return error to call ErrorHandler
	return errors.New("some error")
}

func runFiberApps() {
	app := fiber.New()
	// Create first sub-app with custom error handler.
	one := fiber.New(fiber.Config{
		ErrorHandler: oneErrorHandler,
	})
	one.Get("/some-path", handler)
	app.Mount("/", one)

	// Create second sub-app with custom error handler.
	two := fiber.New(fiber.Config{
		ErrorHandler: twoErrorHandler,
	})
	two.Get("/some-path", handler)
	app.Mount("/two", two)

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

func main() {
	// Create one fiber.App with two sub apps and start listen on :3000.
	go runFiberApps()

	// Multiple calls to the handler "two"
	// to see a random error handler calls.
	for i := 0; i < 20; i++ {
		_, _ = http.Get("http://localhost:3000/two/some-path")

	}

	fmt.Println("the end")
}

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 May 16, 2024

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
Copy link
Member

@DaryaHom your expectations are not entirely correct

https://docs.gofiber.io/guide/grouping
image

since only a flat list exists internally in fiber and your first app is registered on the "root" path, the error handler also works for this path

the problem in the example is that you want to achieve app separation for different paths, but the paths overlap in prefix matching

the registration works like a middleware "app.Use" and uses a prefix matching for all things that do not match exactly, such as the error handler

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

No branches or pull requests

2 participants