Visitar URL original
Failure to create type should be reported as error by snakefoot · Pull Request #6051 · NLog/NLog · GitHub
Skip to content

Failure to create type should be reported as error - #6051

Merged
snakefoot merged 1 commit into
NLog:devfrom
snakefoot:target_config_error
Nov 25, 2025
Merged

Failure to create type should be reported as error#6051
snakefoot merged 1 commit into
NLog:devfrom
snakefoot:target_config_error

Conversation

@snakefoot

@snakefoot snakefoot commented Nov 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Resolves #6050

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

coderabbitai Bot commented Nov 25, 2025 •
edited
Loading

Copy link
Copy Markdown

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

sonarqubecloud Bot commented Nov 25, 2025

Copy link
Copy Markdown

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