Azure-sdk-for-js: Improve Doc phrasing for DefaultAzureCredential.GetToken

[joshfree]

Tracking Work Item

This method is called by Azure SDK clients. It isn't intended for use in application code.

The statement from the XML docs mentioned above, was meant to discourage people from calling GetTokenAsync directly in the most common case that they are using a client from Azure SDK which supports authenticating using a TokenCredential. Let's clarify the message as at least 2 users have been confused by the phrasing above.

Related https://github.com/Azure/azure-sdk-for-net/issues/13531 and https://github.com/Azure/azure-sdk-for-python/issues/13875 and https://github.com/Azure/azure-sdk-for-java/issues/15369

[joshfree]

tagging as blocking-release purely to make this stand out for the October milestone.

/cc @jianghaolu who is looking at this now for Java (we'll want to make this consistent across all 4 languages)

[sadasant]

Here are some specific suggestions by Scott: https://github.com/Azure/azure-sdk-for-net/issues/13531#issuecomment-667481486

[sadasant]

I'll make a PR with:

When using Azure SDK clients, this method is automatically called through TokenCredential and doesn't need to be used in application code.

[sadasant]

So, this is an interesting case,

TypeScript doesn't have this method visible as part of the DefaultAzureCredential, since it's inherited.

So, the DefaultAzureCredential code can be seen here: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/identity/identity/src/credentials/defaultAzureCredential.ts

The getToken method comes from the ChainedTokenCredential: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/identity/identity/src/credentials/chainedTokenCredential.ts#L41-L51

The current documentation in the getToken method of the ChainedTokenCredential reads as follows:

  /**
   * Returns the first access token returned by one of the chained
   * `TokenCredential` implementations.  Throws an {@link AggregateAuthenticationError}
   * when one or more credentials throws an {@link AuthenticationError} and
   * no credentials have returned an access token.
   *
   * @param scopes The list of scopes for which the token will have access.
   * @param options The options used to configure any requests this
   *                `TokenCredential` implementation might make.
   */
  async getToken(

In the context of TypeScript, this makes perfect sense to me, so I wonder, should we still pursue this change? or should we close this issue?

[joshfree]

@sadasant that comment looks perfectly reasonable. The work item is to track getting all 4 languages in-sync on the xml doc comment as at least .NET (and python?) have comments that are unintentionally too scary and lead users down the wrong path in some cases. @jianghaolu can you share more?

[jianghaolu]

The comment we've agreed to change to is

// This method is called automatically by Azure SDK client libraries. You may call this method
// directly, but you must also handle token caching and token refreshing.

@joshfree we currently also don't have the "scary comment" in Java either. We can either add this comment, or not do anything at all.

[joshfree]

@jianghaolu please add the new agreed upon doc comment to Java for consistency