Skip to content

Feature implementation from commits f73b0c6..70300b2 - #1

Open
yashuatla wants to merge 15 commits into
feature-base-1from
feature-head-1
Open

Feature implementation from commits f73b0c6..70300b2#1
yashuatla wants to merge 15 commits into
feature-base-1from
feature-head-1

Conversation

@yashuatla

Copy link
Copy Markdown
Owner

This PR contains changes from a range of commits from the original repository.

Commit Range: f73b0c6..70300b2
Files Changed: 33 (28 programming files)
Programming Ratio: 84.8%

Commits included:

piotrpio and others added 15 commits April 2, 2025 17:15
…connect (nats-io#1846)

This is a follow-up to nats-io#1842 which introduced an issue where `nc.rqch`
could be cleared before reading it in `doReconnect`.
This happened e.g. when an auth error triggered reconnect close to
calling `ForceReconnect` (made apparent by `TestAuthExpiredForceReconnect`
starting to be flaky).

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
…activeThreshold (nats-io#1845)

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Jean-Noël Moyne <jnmoyne@gmail.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Co-authored-by: Jean-Noël Moyne <jnmoyne@gmail.com>
Co-authored-by: Piotr Piotrowski <piotr@synadia.com>
…o#1844)

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Neil Twigg <neil@nats.io>
…et on ctx (nats-io#1850)

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
Comment thread jetstream/test/kv_test.go
expectOk(t, err)
marker := msg.Header.Get(jetstream.MarkerReasonHeader)
if marker != expectedReason {
t.Fatalf("Expected marker to be MaxAge, got %q", marker)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Hardcoded error message string.

Error message hardcodes 'MaxAge' instead of using the expectedReason parameter, causing misleading errors when testing other marker types

Current Code (Diff):

- 			t.Fatalf("Expected marker to be MaxAge, got %q", marker)
+ 			t.Fatalf("Expected marker to be %q, got %q", expectedReason, marker)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
t.Fatalf("Expected marker to be MaxAge, got %q", marker)
t.Fatalf("Expected marker to be %q, got %q", expectedReason, marker)

Comment thread jetstream/test/kv_test.go
expectErr(t, err, jetstream.ErrKeyNotFound)
// check if msg with ttl exists on stream
checkMsgHeaders(t, js, kv, "age", "1s", "")
expectErr(t, err, jetstream.ErrKeyNotFound)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Unreachable code after checkMsgHeaders.

The expectErr call after checkMsgHeaders will never execute since checkMsgHeaders would fail first if the message isn't found

Current Code (Diff):

- 		expectErr(t, err, jetstream.ErrKeyNotFound)

Comment thread nats.go
// the reconnect loop to reconnect immediately
// this means that `ForceReconnect` was called
// before entering doReconnect
if rqch == nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Potential nil pointer dereference in select statement.

The code now properly handles the case where rqch is nil, preventing a runtime panic when the select statement tries to read from it later in the function.

Proposed Code:

	if rqch == nil {
		rqch = make(chan struct{})
		close(rqch)
	}

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

Successfully merging this pull request may close these issues.

4 participants