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.

Sign up for GitHub

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

Jump to bottom

📝 [Proposal]: Add Support for Context in RequestID Middleware #3175

Open
3 tasks done
zhangyongding opened this issue Oct 22, 2024 · 6 comments · May be fixed by #3200
Open
3 tasks done

📝 [Proposal]: Add Support for Context in RequestID Middleware #3175

zhangyongding opened this issue Oct 22, 2024 · 6 comments · May be fixed by #3200
Assignees
@JIeJaitt
Labels
✏️ Feature 📝 Proposal v3
Milestone
v3

Comments

@zhangyongding
Copy link

zhangyongding commented Oct 22, 2024 •
edited by gaby
Loading

Convenient to get the value of requestid from UserContext

// New creates a new middleware handler
func New(config ...Config) fiber.Handler {
	// Set default config
	cfg := configDefault(config...)

	// Return new handler
	return func(c fiber.Ctx) error {
		// Don't execute middleware if Next returns true
		if cfg.Next != nil && cfg.Next(c) {
			return c.Next()
		}
		// Get id from request, else we generate one
		rid := c.Get(cfg.Header)
		if rid == "" {
			rid = cfg.Generator()
		}

		// Set new id to response header
		c.Set(cfg.Header, rid)

		// Add the request ID to locals
		c.Locals(requestIDKey, rid)

		// Add the request ID to UserContext
		ctx := context.WithValue(c.UserContext(), cfg.Header, rid)
		c.SetUserContext(ctx)

		// Continue stack
		return c.Next()
	}
}

Alignment with Express API

N/a

HTTP RFC Standards Compliance

N/a

API Stability

N/a

Feature Examples

N/a

Checklist:

  • I agree to follow Fiber's Code of Conduct.
  • I have searched for existing issues that describe my proposal before opening this one.
  • I understand that a proposal that does not meet these guidelines may be closed without explanation.
@ReneWerner87
Copy link
Member

can you describe what advantages it has over the value we already have in the locals?

@zhangyongding
Copy link
Author

Helps to include RequestID when obtaining UserContext, hoping to still carry RequestID when using UserContext as context in some scenarios

@ReneWerner87
Copy link
Member

what do you think we should expand the context in all middlewares where we also fill the locals?
@gaby @efectn

i wouldn't do it only in the requestid mw, because then it wouldn't be consistent

@JIeJaitt
Copy link

@ReneWerner87 @gaby @efectn I think requestid, keyauth, csrf, session middleware may need this function more, cache, compress, logger, limiter, favicon, pprof, recover, redirect, rewrite, etag, cors and helmet, idempotency, earlydata, encryptcookie may not be so need this function. Maybe we can add those with higher needs first.

@ReneWerner87
Copy link
Member

ok can you support us with a PR

@JIeJaitt
Copy link

@ReneWerner87 I'm very happy to finish it. I will finish this PR in the next few days.

@JIeJaitt JIeJaitt linked a pull request Nov 12, 2024 that will close this issue
Open
@gaby gaby added this to v3 Nov 12, 2024
@gaby gaby moved this to In Progress in v3 Nov 12, 2024
@gaby gaby added this to the v3 milestone Nov 12, 2024
@gaby gaby changed the title 📝 [Proposal]: Add support for UserContext in requestid middleware 📝 [Proposal]: Add Support for Context in RequestID Middleware Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JIeJaitt JIeJaitt

Labels
✏️ Feature 📝 Proposal v3
Projects
v3
Status: In Progress
Milestone
v3
Development

Successfully merging a pull request may close this issue.

🔥 feat: Add Context Support to RequestID Middleware JIeJaitt/fiber
4 participants
@gaby @ReneWerner87 @zhangyongding @JIeJaitt