[storage-blob]Refine behavior of BlobClient.exists

[EmmaZhu]

Against a blob uploaded with CPK, storage server would return error message of 'BlobUsesCustomerSpecifiedEncryption' for get attributes (without CPK specified) when blob exists.

BlobClient.exists leverages getAttributes operation to verify blob's existence, based on the above behavior, it should return true if got error message 'BlobUsesCustomerSpecifiedEncryption' from server.

[ramya-rao-a]

@EmmaZhu This PR has not seen any activity in about 4 months. Is this still applicable? Looks like there are breaking changes here

[EmmaZhu]

Thanks @ramya-rao-a for the reminding.

The change is still applicable. I'll handle it later in this week.

Thanks
Emma

[maorleger]

@lmolkova I forget what we decided with regards to "expected exceptions" - should the spans be marked as error or success?

In this case there are two scenarios:

1. 404 when checking if a blob exists, returning false as it does not
2. 409 when checking if a blob exists which is expected as well if the blob uses specified encryption message (not sure what that means, but seems expected here) and returning true

What would we expect the span status to be? And IIRC the nested HTTP spans will return error since it's a 400ish status code.

[lmolkova]

We should avoid setting span status for 404/409 calls at all if we are not sure an error has happened:

• the http status code is enough for Http calls.
• Logical calls can still end without a status set at all and users can figure out what happened based on the underlying http span.

There could be some room for improvement here (e.g. we might need to show what exists eventually returned), but it feels like an incremental change as long as we leave status unset.

We must not add an exception in any case.

[EmmaZhu]

This is a convenience existence checking for customer.

We only have logic to understand the status we exactly know the meanings.
404 means the target doesn't exist, so we return false.
409 means the target exists, but the provided encryption key is not valid, but existence checking doesn't care about encryption, so we return true to tell customer the target exists.

We won't catch other reported errors.

[maorleger]

In this case then I would recommend the following:

• In the success case (line 1273) set the span status to success
• In the error case (the catch block), if the http statusCode is 404 or 409 leave the span status unset, otherwise set it to error and record the message (as you are already doing in line 1291)

Something like this:

  public async exists(options: BlobExistsOptions = {}): Promise<boolean> {
    const { span, updatedOptions } = createSpan("BlobClient-exists", options);
    try {
      ensureCpkIfSpecified(options.customerProvidedKey, this.isHttps);
      await this.getProperties({
        abortSignal: options.abortSignal,
        customerProvidedKey: options.customerProvidedKey,
        conditions: options.conditions,
        tracingOptions: updatedOptions.tracingOptions,
      });
      span.setStatus({
        code: SpanStatusCode.OK,
      });
      return true;
    } catch (e) {
      if (e.statusCode === 404) {
        return false;
      } else if (
        e.statusCode === 409 &&
        e.details.errorCode === BlobUsesCustomerSpecifiedEncryptionMsg
      ) {
        return true;
      }

      span.setStatus({
        code: SpanStatusCode.ERROR,
        message: e.message,
      });
      throw e;
    } finally {
      span.end();
    }
  }

[EmmaZhu]

Thanks a lot for the comments. I've made change as you suggested.

[XiaoningLiu]

As synchronized offline, when blob doesn't exist, it will never return 409. This change is also aligned with .Net SDK.