Skip to content

BufferingTargetWrapper - Improve InternalLogger output when WrappedTarget is NULL#6062

Merged
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:WrapperAlert
Dec 14, 2025
Merged

BufferingTargetWrapper - Improve InternalLogger output when WrappedTarget is NULL#6062
snakefoot merged 1 commit intoNLog:devfrom
snakefoot:WrapperAlert

Conversation

@snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Dec 14, 2025 •
edited
Loading

NLog already alerts when initialization fails, no need to repeat the same error over and over.

@snakefoot snakefoot added the enhancement Improvement on existing feature label Dec 14, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 14, 2025 •
edited
Loading

Walkthrough

Two logging wrapper classes have been updated to downgrade the severity of null WrappedTarget conditions from Error to Debug level and adjust the corresponding log message to indicate missing output. No control flow modifications.

Changes

Cohort / File(s) Summary
Logging Level Adjustments for Null WrappedTarget
src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, src/NLog/Targets/Wrappers/BufferingTargetWrapper.cs		 Log level downgraded from Error to Debug when WrappedTarget is null; log message updated from "WrappedTarget is NULL" to "No output because WrappedTarget is NULL" in WriteLogEventsInQueue and WriteEventsInBuffer methods respectively.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Focused change in two similar wrapper classes affecting only logging statements
  • No logic, control flow, or API surface changes
  • Message clarification is straightforward and intentional severity downgrade

Poem

🐰 A whisper instead of a shout,
When targets take their bow and step out,
Debug logs the news, calm and serene—
No fuss, no fury, just what it means! 🌙

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to evaluate whether it relates to the changeset. Add a pull request description explaining the rationale for downgrading log levels and updating log messages when WrappedTarget is NULL.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: improving InternalLogger output in BufferingTargetWrapper when WrappedTarget is NULL, which matches the changeset modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Dec 14, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/NLog/Targets/Wrappers/BufferingTargetWrapper.cs (1)

317-321: Downgrading NULL WrappedTarget to Debug looks reasonable; consider optionally including reason for diagnosis.
The updated message is clearer (“no output”) and Debug reduces internal-log noise for wrapper lifecycles. If you want a bit more forensic value when Debug is enabled, consider appending the reason (when non-empty) to the message.

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs (1)

561-565: Consistent, lower-noise internal logging for NULL WrappedTarget; message improvement is good.
The Debug-level log plus “No output…” wording better reflects the outcome without implying a runtime failure. (Optional) include reason to help correlate where/why the flush happened when Debug is enabled.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4dd16e8 and 5d571a2.

📒 Files selected for processing (2)
  • src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs (1 hunks)
  • src/NLog/Targets/Wrappers/BufferingTargetWrapper.cs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs (3)
src/NLog/Common/InternalLogger.cs (1)
  • InternalLogger (51-511)
src/NLog/Logger-generated.cs (5)
  • Debug (403-409)
  • Debug (417-423)
  • Debug (429-436)
  • Debug (457-463)
  • Debug (515-521)
src/NLog/Config/InstallationContext.cs (1)
  • Debug (121-124)
src/NLog/Targets/Wrappers/BufferingTargetWrapper.cs (1)
src/NLog/Common/InternalLogger.cs (1)
  • InternalLogger (51-511)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 14, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@snakefoot snakefoot merged commit 624863f into NLog:dev Dec 14, 2025
5 of 6 checks passed
@snakefoot snakefoot added this to the 6.1.0 milestone Jan 31, 2026
@snakefoot snakefoot changed the title BufferingTargetWrapper - Improve InternaLogger output when WrappedTarget is NULL BufferingTargetWrapper - Improve InternalLogger output when WrappedTarget is NULL Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

enhancement Improvement on existing feature size/XS

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

1 participant

@snakefoot