Skip to content

Fix notifier Ping called with cancelled context#1149

Merged
brandur merged 1 commit intoriverqueue:masterfrom
GiGurra:fix/notifier-ping-cancelled-ctx
Feb 15, 2026
Merged

Fix notifier Ping called with cancelled context#1149
brandur merged 1 commit intoriverqueue:masterfrom
GiGurra:fix/notifier-ping-cancelled-ctx

Conversation

@GiGurra
Copy link
Contributor

@GiGurra GiGurra commented Feb 13, 2026

Summary

  • In waitOnce, the inner context is cancelled by drainErrChan to interrupt WaitForNotification, but the subsequent Ping call was using that same cancelled context, meaning it would always fail with context.Canceled.
  • Fix: save a reference to the parent (still-live) context before creating the inner cancellable one, and use it for the Ping call.
  • Extends ListenerMock with a pingFunc to enable testing Ping behavior, and adds a testPingInterval field to avoid the 5s wait in tests.

Test plan

  • New test TestNotifier/PingUsesNonCancelledContext verifies Ping receives a non-cancelled context
  • Test confirmed to fail without the fix (Ping receives cancelled context) and pass with the fix
  • Full notifier test suite passes (18/18 tests)

🤖 Generated with Claude Code

In waitOnce, the inner context is cancelled by drainErrChan to interrupt
WaitForNotification, but the subsequent Ping call was using that same
cancelled context, meaning it would always fail with context.Canceled.

Save a reference to the parent (still-live) context before creating the
inner cancellable context, and use it for the Ping call.

Also adds a testPingInterval field to avoid the 5s wait in tests, and
extends ListenerMock with a pingFunc for testing Ping behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GiGurra GiGurra force-pushed the fix/notifier-ping-cancelled-ctx branch from 6fef647 to 3121667 Compare February 13, 2026 20:22
@brandur brandur merged commit 9e19929 into riverqueue:master Feb 15, 2026
11 checks passed
brandur added a commit that referenced this pull request Feb 15, 2026
Adds changelog entries for #1147, #1148, and #1149.

Also make a couple small tweaks: one to alphabetize a newly added
property, and one to move a success case test to above the error case,
which is a little more conventional.
brandur added a commit that referenced this pull request Feb 15, 2026
Adds changelog entries for #1147, #1148, and #1149.

Also make a couple small tweaks: one to alphabetize a newly added
property, and one to move a success case test to above the error case,
which is a little more conventional.
brandur added a commit that referenced this pull request Feb 15, 2026
Adds changelog entries for #1147, #1148, #1149, and #1150.

Also make a couple small tweaks: one to alphabetize a newly added
property, and one to move a success case test to above the error case,
which is a little more conventional.
brandur added a commit that referenced this pull request Feb 15, 2026
#1151)

Adds changelog entries for #1147, #1148, #1149, and #1150.

Also make a couple small tweaks: one to alphabetize a newly added
property, and one to move a success case test to above the error case,
which is a little more conventional.
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.

2 participants