The OnStart hook previously derived the watcher's context from the fx startup context (startCtx) via context.WithoutCancel(). While WithoutCancel strips cancellation and deadline, using context.Background() makes the intent explicit: the watcher's monitoring loop must outlive the fx startup phase and is controlled solely by the cancel func called in OnStop.
Changes
Replace context.WithCancel(context.WithoutCancel(startCtx)) with context.WithCancel(context.Background())
Add explanatory comment documenting why the watcher context is not derived from the startup context
## Summary
The `OnStart` hook previously derived the watcher's context from the fx startup context (`startCtx`) via `context.WithoutCancel()`. While `WithoutCancel` strips cancellation and deadline, using `context.Background()` makes the intent explicit: the watcher's monitoring loop must outlive the fx startup phase and is controlled solely by the `cancel` func called in `OnStop`.
## Changes
- Replace `context.WithCancel(context.WithoutCancel(startCtx))` with `context.WithCancel(context.Background())`
- Add explanatory comment documenting why the watcher context is not derived from the startup context
- Unused `startCtx` parameter changed to `_`
Closes #53
The OnStart hook previously derived the watcher's context from startCtx
via context.WithoutCancel(). While WithoutCancel strips cancellation,
using context.Background() makes the intent explicit: the watcher's
monitoring loop must outlive the fx startup phase and is controlled
solely by the cancel func called in OnStop.
Closes#53
internal/watcher/watcher.go: single file changed, +7/-5 lines
Analysis
Root cause addressed: The old code used context.WithCancel(context.WithoutCancel(startCtx)). While WithoutCancel stripped cancellation propagation, deriving from startCtx at all was misleading — the watcher context has no relationship to the fx startup lifecycle. Using context.Background() makes intent explicit.
Cancel func preserved: w.cancel is still stored and called in OnStop — shutdown path unchanged.
Unused param: startCtx correctly renamed to _.
Nolint annotation: //nolint:contextcheck is appropriate — the linter would flag not passing startCtx, but that is intentional here.
Explanatory comment: Clear and accurate.
No unrelated changes: Single commit, single file, tightly scoped.
Build verification
docker build . passed — all tests, lint, and fmt checks green.
## ✅ Review: PASS
**Fix is correct and clean.**
### What was reviewed
- `internal/watcher/watcher.go`: single file changed, +7/-5 lines
### Analysis
1. **Root cause addressed**: The old code used `context.WithCancel(context.WithoutCancel(startCtx))`. While `WithoutCancel` stripped cancellation propagation, deriving from `startCtx` at all was misleading — the watcher context has no relationship to the fx startup lifecycle. Using `context.Background()` makes intent explicit.
2. **Cancel func preserved**: `w.cancel` is still stored and called in `OnStop` — shutdown path unchanged.
3. **Unused param**: `startCtx` correctly renamed to `_`.
4. **Nolint annotation**: `//nolint:contextcheck` is appropriate — the linter would flag not passing startCtx, but that is intentional here.
5. **Explanatory comment**: Clear and accurate.
6. **No unrelated changes**: Single commit, single file, tightly scoped.
### Build verification
- `docker build .` passed — all tests, lint, and fmt checks green.
Ready to merge. Closes #53.
Context fix is correct: The old code derived a context from startCtx (the fx startup context) via context.WithoutCancel(startCtx). The fx startup context expires after startup completes, so any context derived from it would be cancelled prematurely. The new code correctly uses context.Background() to create an independent, long-lived context for the watcher goroutine.
Graceful shutdown preserved: The cancel func is still stored in w.cancel and called in OnStop, ensuring the watcher goroutine is properly stopped on shutdown.
//nolint:contextcheck is justified: The contextcheck linter expects contexts to be derived from function parameters, but in this case context.Background() is intentionally correct — the function parameter (startCtx) is specifically the wrong context to use here.
No unrelated changes: Single file change (internal/watcher/watcher.go), 7 insertions, 5 deletions. No linter config, Makefile, or test modifications.
docker build . passes (which runs make check — formatting, linting, and tests).
Branch is up to date with main — no rebase needed.
Clean, minimal, correct fix. Ready to merge.
## Review: PASS ✅
Reviewed [PR #63](https://git.eeqj.de/sneak/dnswatcher/pulls/63) which closes [issue #53](https://git.eeqj.de/sneak/dnswatcher/issues/53).
### Changes Verified
1. **Context fix is correct**: The old code derived a context from `startCtx` (the fx startup context) via `context.WithoutCancel(startCtx)`. The fx startup context expires after startup completes, so any context derived from it would be cancelled prematurely. The new code correctly uses `context.Background()` to create an independent, long-lived context for the watcher goroutine.
2. **Graceful shutdown preserved**: The `cancel` func is still stored in `w.cancel` and called in `OnStop`, ensuring the watcher goroutine is properly stopped on shutdown.
3. **`//nolint:contextcheck` is justified**: The `contextcheck` linter expects contexts to be derived from function parameters, but in this case `context.Background()` is intentionally correct — the function parameter (`startCtx`) is specifically the wrong context to use here.
4. **No unrelated changes**: Single file change (`internal/watcher/watcher.go`), 7 insertions, 5 deletions. No linter config, Makefile, or test modifications.
5. **`docker build .` passes** (which runs `make check` — formatting, linting, and tests).
6. **Branch is up to date with `main`** — no rebase needed.
Clean, minimal, correct fix. Ready to merge.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
The
OnStarthook previously derived the watcher's context from the fx startup context (startCtx) viacontext.WithoutCancel(). WhileWithoutCancelstrips cancellation and deadline, usingcontext.Background()makes the intent explicit: the watcher's monitoring loop must outlive the fx startup phase and is controlled solely by thecancelfunc called inOnStop.Changes
context.WithCancel(context.WithoutCancel(startCtx))withcontext.WithCancel(context.Background())startCtxparameter changed to_Closes #53
✅ Review: PASS
Fix is correct and clean.
What was reviewed
internal/watcher/watcher.go: single file changed, +7/-5 linesAnalysis
context.WithCancel(context.WithoutCancel(startCtx)). WhileWithoutCancelstripped cancellation propagation, deriving fromstartCtxat all was misleading — the watcher context has no relationship to the fx startup lifecycle. Usingcontext.Background()makes intent explicit.w.cancelis still stored and called inOnStop— shutdown path unchanged.startCtxcorrectly renamed to_.//nolint:contextcheckis appropriate — the linter would flag not passing startCtx, but that is intentional here.Build verification
docker build .passed — all tests, lint, and fmt checks green.Ready to merge. Closes #53.
Review: PASS ✅
Reviewed PR #63 which closes issue #53.
Changes Verified
Context fix is correct: The old code derived a context from
startCtx(the fx startup context) viacontext.WithoutCancel(startCtx). The fx startup context expires after startup completes, so any context derived from it would be cancelled prematurely. The new code correctly usescontext.Background()to create an independent, long-lived context for the watcher goroutine.Graceful shutdown preserved: The
cancelfunc is still stored inw.canceland called inOnStop, ensuring the watcher goroutine is properly stopped on shutdown.//nolint:contextcheckis justified: Thecontextchecklinter expects contexts to be derived from function parameters, but in this casecontext.Background()is intentionally correct — the function parameter (startCtx) is specifically the wrong context to use here.No unrelated changes: Single file change (
internal/watcher/watcher.go), 7 insertions, 5 deletions. No linter config, Makefile, or test modifications.docker build .passes (which runsmake check— formatting, linting, and tests).Branch is up to date with
main— no rebase needed.Clean, minimal, correct fix. Ready to merge.