Skip to content

Failure to create type should be reported as error #6051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
snakefoot merged 1 commit into from
Nov 25, 2025
Merged

Failure to create type should be reported as error #6051

snakefoot merged 1 commit into from
Nov 25, 2025
+31 −0

Conversation

@snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Nov 25, 2025 •
edited
Loading

Resolves #6050

@snakefoot snakefoot added the enhancement Improvement on existing feature label Nov 25, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025 •
edited
Loading

Walkthrough

These changes enhance error reporting in NLog's configuration parsing by adding internal error logging when factory instance creation fails, and introduce a unit test verifying error messages are captured for unknown target types.

Changes

Cohort / File(s) Summary
Error logging in configuration parser
src/NLog/Config/LoggingConfigurationParser.cs		 Adds InternalLogger.Error calls in FactoryCreateInstance exception handlers to log NLogConfigurationException and wrapped general exceptions with details about the target type
Unknown target error reporting test
tests/NLog.UnitTests/Config/XmlConfigTests.cs		 Adds new unit test UnknownTargetTypeTest_ReportError that validates error logging occurs when an unknown target type is encountered during XML configuration parsing

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Exception handler logging additions are straightforward and non-invasive
  • New test is self-contained with clear assertions
  • No complex logic or control-flow modifications

Poem

🐰 A rabbit hops through parsing logs,
Where errors now are seen,
Unknown targets cause no fog—
They're logged and caught between!
ErrorReporting, bug-free dreams. 🎯

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 pull request description was provided by the author; the PR relies solely on the issue title for context. Add a pull request description explaining the changes, why they were made, and how they address the issue about reporting type creation failures as errors.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding error logging when type creation fails in the FactoryCreateInstance method.
✨ 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.

@snakefoot snakefoot mentioned this pull request Nov 25, 2025
Closed
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 25, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@snakefoot snakefoot merged commit 16d79de into NLog:dev Nov 25, 2025
4 of 6 checks passed
@snakefoot snakefoot added this to the 6.0.7 milestone Nov 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement Improvement on existing feature size/M

Projects

None yet

Milestone

6.0.7

Development

Successfully merging this pull request may close these issues.

NLog.NLogConfigurationException: BufferingTargetWrapper(Name=mailbuffer): No wrapped Target configured.

1 participant

@snakefoot