Skip to content
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

Exception class naming/structure/usage creates confusion #610

Open
kevbry opened this issue May 30, 2022 · 1 comment
Open

Exception class naming/structure/usage creates confusion #610

kevbry opened this issue May 30, 2022 · 1 comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap

Comments

@kevbry
Copy link

kevbry commented May 30, 2022

Issue Summary

Because of naming, protection level and class hierarchy, it is not obvious what exceptions could possibly be thrown by the SDK.

Twilio.Exceptions contains 6 classes that either directly or indirectly extend System.Exception.

TwilioException.cs

  • TwilioException : Exception
    • Marker base class for all library exceptions
  • ApiConnectionException : TwilioException
    • Wraps any exception thrown by the underlying HttpClient. Sets message to endpoint URL attempted.
  • AuthenticationException : TwilioException
    • Thrown if username or password has not been set

CertificateValidationException.cs

  • CertificateValidationException : TwilioException
    • Unknown usage

RestException.cs

  • RestException : TwilioException
    • Docs mark this as "Exception from Twilio API". Is actually a DTO representing the JSON error response received from the API, and never thrown despite inheriting from Exception. Only ever used within the context of a private method in TwilioRestClient.

ApiException.cs

  • ApiException : TwilioException
    • Docs mark this as a POCO representing an API exception. Is actually used as the exception type thrown when an error response is returned by a Twilio API.

This structure created a few issues for me:

  • ApiException docs make it appear that it's just a POCO, even though it's actually the primary exception type you'd need to catch when interacting with the SDK. It also seems to be the only exception mentioned by SDK documentation, even though others exist in common scenarios (haven't called Init() yet).
  • ApiException sometimes contains the HTTP status code of the response when one was received, but sometimes does not (https://github.com/twilio/twilio-csharp/blob/main/src/Twilio/Clients/TwilioRestClient.cs#L194 has access to the HTTP status code when thrown an ApiException, but does not set the status code on the exception)
  • RestException is a public subclass of TwilioException and Exception, making it appear as though it'd be an exception thrown by the SDK. It's only ever used in a private method in TwilioRestClient as a DTO, so it should probably be either private or internal so as to not cause confusion, especially since it has a property set that's identical to ApiException.
  • There isn't consistency in where these classes are located. TwilioException.cs contains three classes that are all basic Exception wrappers, but CertificateValidationException is also a basic wrapper around Exception and is in its own file. It makes the code a bit difficult to navigate.
@claudiachua claudiachua added type: question question directed at the library status: waiting for feedback waiting for feedback from the submitter labels Jun 3, 2022
@claudiachua
Copy link
Contributor

Hi @kevbry , Thanks for bringing this to our attention. You can either open a PR to update the docs/exceptions and our team will review it based on our review backlog or I can add this to our internal backlog to be prioritized

@JenniferMah JenniferMah added type: community enhancement feature request not on Twilio's roadmap status: help wanted requesting help from the community and removed type: question question directed at the library status: waiting for feedback waiting for feedback from the submitter labels Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

No branches or pull requests

3 participants