-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
AttachContext is not transitive #67
Comments
Hi @rantav, If you create a second context with value, you must use So must do: // First time
ctx2 := context.WithValue(ctx, "k2", "v2")
ctx.AttachContext(ctx2)
// Second time or more
ctx3 := context.WithValue(ctx.AttachedContext(), "k3", "v3")
ctx.AttachContext(ctx3) |
Thanks @savsgio I understand your point. Let me try to add another angle if I may.
ctx := acquireRequestCtx(new(fasthttp.RequestCtx))
ctx.SetUserValue("k1", "v1") // ctx contains k1
ctx2 := context.WithValue(ctx, "k2", "v2")
ctx.AttachContext(ctx2) // ctx contains k1, k2
// I want ctx3 to wrap both k1 and k2, but I can't.
// Option 1
ctx3 := context.WithValue(ctx, "k3", "v3") // ctx3 contains k3, k1 and k2
// but if we do that: surprise
ctx.AttachContext(ctx3) // now ctx contains k1, k3 (not k2)
// Option 2 (instead of 1)
ctx3 := context.WithValue(ctx2, "k3", "v3") // ctx3 contains k3, k2
// And now, surprise
ctx.AttachContext(ctx3) // now ctx contains k1, k3 (not k2) |
I'm sorry, but your example is good, the fmt.Println(ctx.Value("k1"), ctx.Value("k2"), ctx.Value("k3"))
# Print: v1 v2 v3 Keep in mind, that you could only attach one context, if you attach another, you override the previous, that's why to use the Maybe in a future, @erikdubbelboer or @kirillDanshin will implement it, but now, it's not possible. |
ok, I get it, it's a limitation or "by design" of fasthttp. |
Sorry, use attached context carefully. |
I realized there is another problem with AttachContext and friends.
Attaching a context and then creating a new context and attaching it, makes
RequestCtx
forget the previous attached context, which to me seems like a breach of the context contractExample test:
I'll send a pull request with fix suggestion soon
The text was updated successfully, but these errors were encountered: