[test] Remove dead nil guards and unreachable else branch in logger/jsonl_logger.go - #9714
Merged
lpcox merged 1 commit intoJul 20, 2026
Conversation
The withGlobalLogger helper already guarantees a non-nil logger before invoking any callback (see global_helpers.go:127). The nil guard checks inside the LogDifcFilteredItem and LogUnrecognizedEndpointPassthrough callbacks were therefore dead code — they could never be reached. Also simplify json.Marshal for JSONLUnrecognizedEndpointPassthrough: the struct has only string fields, so marshaling cannot fail. Replace the if/else error pattern with b, _ := json.Marshal(entry) to match similar patterns elsewhere (e.g. rpc_logger.go). These changes are consistent with all other withGlobalLogger callbacks in the package (rpc_logger.go, observed_url_domains_logger.go) which do not include redundant nil guards. Coverage improvement: LogDifcFilteredItem 88.9% → 100%, LogUnrecognizedEndpointPassthrough 75% → 100% Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes unreachable defensive branches from JSONL logging callbacks.
Changes:
- Removes redundant nil guards already enforced by
withGlobalLogger. - Simplifies marshaling of a string-only audit entry.
Show a summary per file
| File | Description |
|---|---|
internal/logger/jsonl_logger.go |
Removes dead branches from two logging paths. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Medium
lpcox
deleted the
test-coverage/remove-dead-nil-guards-jsonl-logger-206e89da73c72731
branch
July 20, 2026 18:12
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Code Quality Improvement: Dead Code Removal in
jsonl_logger.goWhat Changed
internal/loggerinternal/logger/jsonl_logger.goLogDifcFilteredItem,LogUnrecognizedEndpointPassthroughLogDifcFilteredItem88.9%,LogUnrecognizedEndpointPassthrough75.0%Why These Changes?
The
withGlobalLoggerhelper (inglobal_helpers.go:127) already guarantees a non-nil logger before invoking any callback:The
if logger == nil { return }guards inside theLogDifcFilteredItemandLogUnrecognizedEndpointPassthroughcallbacks were therefore unreachable dead code — the callback is only ever called with a guaranteed non-nil logger.This is inconsistent with all other
withGlobalLoggercallbacks in the codebase (rpc_logger.go,observed_url_domains_logger.go) which do not have redundant nil guards.Additional Simplification
LogUnrecognizedEndpointPassthroughalso had anif/elsepattern forjson.Marshalwhere theelsebranch was unreachable:JSONLUnrecognizedEndpointPassthroughhas onlystringfields, so marshaling cannot fail. Replaced withb, _ := json.Marshal(entry), matching the pattern inrpc_logger.go.Tests
Existing tests in
jsonl_logger_test.gocontinue to pass and cover both functions:TestLogDifcFilteredItem_NoLogger— verifies no panic when logger is uninitializedTestLogDifcFilteredItem_WritesAuditEntryToJSONL— verifies correct JSONL outputTestLogUnrecognizedEndpointPassthrough_NoLogger— verifies no panic when logger is uninitializedTestLogUnrecognizedEndpointPassthrough_WritesCorrectFields— verifies correct JSONL outputCoverage Report
Generated by Test Coverage Improver
Warning
Firewall blocked 5 domains
The following domains were blocked by the firewall during workflow execution:
awmgmcpggoproxy.cngoproxy.ioproxy.golang.orgreleaseassets.githubusercontent.comSee Network Configuration for more information.