Azure-sdk-for-js: [Identity] Should CredentialUnavailable be called CredentialUnavailableError?

[sadasant]

At the moment, errors CredentialUnavailable and AuthenticationRequired in Identity do not end in Error. Should they?

CredentialUnavailable was GAd with v1, but we could do this change since we're releasing v2 soon.

This issue is only about the name of those errors!

[ramya-rao-a]

We generally do have the Error suffix on the custom error classes in Identity and in other packages. In Identity itself, we already have AuthenticationError and AggregateAuthenticationError.

I would definitely recommend AuthenticationRequired that was introduced in v2 beta to be renamed to AuthenticationRequiredError

For the CredentialUnavailable case, I don't mind renaming it to CredentialUnavailableError. We are doing a major version update and we can document it as a breaking change. Moreover, user's tooling will surface the error as the old class is no longer exported and they can react appropriately.

My concern is around the "name". I would hesitate to change the name on the CredentialUnavailable class to also get the "Error" suffix. This is because no tooling will catch any problems with this old code if (err.name === "CredentialUnavailable")

@maorleger, @bterlson Thoughts?

[ramya-rao-a]

My concern is around the "name". I would hesitate to change the name on the CredentialUnavailable class to also get the "Error" suffix. This is because no tooling will catch any problems with this old code if (err.name === "CredentialUnavailable")

But this might also end up confusing users who might see the class CredentialUnavailableError and expect the name to also be CredentialUnavailableError. :(

[sadasant]

Let's also get @bterlson or @xirzec here to see one additional perspective. We have some time (couple of weeks) to make this decision.

[xirzec]

I like the general rule of Errors having constructor names that end in Error. I strongly think the name property should match the constructor name. I am supportive of renaming errors across a major version change, assuming we rename both the property and the constructor.