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] metadata returns old results when key is not title case #2738

Open
xhd2015 opened this issue Nov 3, 2024 · 2 comments
Open

[BUG] metadata returns old results when key is not title case #2738

xhd2015 opened this issue Nov 3, 2024 · 2 comments

Comments

@xhd2015
Copy link

xhd2015 commented Nov 3, 2024

Describe the bug

In our unit test we have a test that checks update of a metadata context yields expected result when Get. But it fails randomly.

After investigation, we think the bug is related to metadata package, and possibly due to go's random map traversing behavior.

But I am not sure if this is considered a bug or not, so raise an issue here to discuss.

How to reproduce the bug

  • Description: The following test sets metadata with the same key twice,
    • first "A", then "a",
    • and then get the key from the metadata back,
    • The result is always expected to be "a", but it sometimes returns "A".
  • Observation: this test usually fails within 10 loops
package demo

import (
	"context"
	"testing"

	"go-micro.dev/v5/metadata"
)

// go.mod: require go-micro.dev/v5 v5.3.0

const LOWER_TEST_KEY = "lower-test-key"

func TestMetadataBug(t *testing.T) {
	n := 0
	for {
		ctx := context.TODO()
		ctx1 := metadata.Set(ctx, LOWER_TEST_KEY, "A")

		ctx2 := metadata.Set(ctx1, LOWER_TEST_KEY, "a")

		v, _ := metadata.Get(ctx2, LOWER_TEST_KEY)
		if v != "a" {
			t.Fatalf("fail: %d, found unexpected result: %v", n, v)
		}
		t.Logf("pass: %d", n)
		n++
	}
}

Output:

=== RUN   TestMetadataBug
    v2_test.go:24: pass: 0
    v2_test.go:24: pass: 1
    v2_test.go:24: pass: 2
    v2_test.go:24: pass: 3
    v2_test.go:24: pass: 4
    v2_test.go:24: pass: 5
    v2_test.go:24: pass: 6
    v2_test.go:24: pass: 7
    v2_test.go:24: pass: 8
    v2_test.go:24: pass: 9
    v2_test.go:24: pass: 10
    v2_test.go:24: pass: 11
    v2_test.go:22: fail: 12, found unexpected result: A
--- FAIL: TestMetadataBug (0.00s)

Here is the runnable snippet: https://go.dev/play/p/h_55BgcL6wY (need to place require go-micro.dev/v5 v5.3.0 in your go.mod first)

Environment

Go Version: go version go1.19.6 darwin/amd64

@xhd2015
Copy link
Author

xhd2015 commented Nov 3, 2024

Possible reason:

The Set function does not convert lower case to title case, so at a point the metadata map may hold both the lower case and the title case key:

} else {
md[k] = v
}

And in FromContext, there is a map copying via traversing, which is randomized, the actual value depends on which key comes at last:

for k, v := range md {
newMD[strings.Title(k)] = v
}

@xhd2015
Copy link
Author

xhd2015 commented Nov 3, 2024

Sequence:
image

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

No branches or pull requests

1 participant