Skip to content

Doc updates#21

Open
tomerqodo wants to merge 2 commits intosentry_combined_20260121_augment_sentry_coderabbit_1_base_doc_updates_pr96from
sentry_combined_20260121_augment_sentry_coderabbit_1_head_doc_updates_pr96
Open

Doc updates#21
tomerqodo wants to merge 2 commits intosentry_combined_20260121_augment_sentry_coderabbit_1_base_doc_updates_pr96from
sentry_combined_20260121_augment_sentry_coderabbit_1_head_doc_updates_pr96

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from qodo-benchmark#96

Comment on lines +47 to 50
if (value <= 0)
{
throw new ArgumentOutOfRangeException(nameof(value), $"{nameof(BackgroundQueueSize)} must be non-negative.");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The validation for BackgroundQueueSize rejects 0, but the error message incorrectly states the value can be non-negative, creating a contradiction and a breaking API change.
Severity: MEDIUM

Suggested Fix

Update the error message for the BackgroundQueueSize property validation. Change the string from "{nameof(BackgroundQueueSize)} must be non-negative." to "{nameof(BackgroundQueueSize)} must be positive." to align with the validation logic that rejects zero.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/Logging.AzureAppServices/src/BatchingLoggerOptions.cs#L47-L50

Potential issue: The validation for `BackgroundQueueSize` was changed to reject a value
of `0`, but the corresponding error message was not updated. The code now throws an
`ArgumentOutOfRangeException` if the value is `0`, while the error message incorrectly
states that the value "must be non-negative" (>= 0). This creates a contradiction and is
a breaking change for a public API that previously accepted `0`. This behavior is also
inconsistent with similar properties like `BatchSize` in the same class, which use the
message "must be positive" when rejecting zero.

Did we get this right? 👍 / 👎 to inform future reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants