Skip to content

Better exception messages #3699

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.

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

Closed
wants to merge 1 commit into from
Closed

Conversation

opottone
Copy link

Improve error messages.

Description

In some cases when the SDK throws an exception, the error message is "Value cannot be null. (Parameter 'Options property cannot be empty: ClientName')". It should be "Value cannot be null. (Parameter 'ClientName')". The problem is caused by incorrect usage of ArgumentNullException constructor.

Motivation and Context

Testing

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

This `ArgumentNullException` constructor takes as its argument the name
of the offending parameter, not a free form error message. See
https://learn.microsoft.com/en-us/dotnet/api/system.argumentnullexception.-ctor?view=net-9.0#system-argumentnullexception-ctor(system-string)
@dscpinheiro dscpinheiro changed the base branch from main to main-staging March 11, 2025 10:31
@dscpinheiro
Copy link
Contributor

dscpinheiro commented Mar 11, 2025

Thanks for the contribution, but as you noticed (in #3704) this class has a bigger problem where it doesn't handle expired credentials, and updating the error message doesn't really improve things that much.

I'll close this PR and we'll triage that issue instead.

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