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

azblob: wrong example in error casting for StorageError #16920

Closed
bcho opened this issue Jan 26, 2022 · 1 comment
Closed

azblob: wrong example in error casting for StorageError #16920

bcho opened this issue Jan 26, 2022 · 1 comment
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Docs Storage Storage Service (Queues, Blobs, Files)

Comments

@bcho
Copy link
Member

bcho commented Jan 26, 2022

Bug Report

  • import path of package in question:.../sdk/storage/azblob
  • SDK version: main
  • output of go version: go version go1.17.3 darwin/amd64
  • What happened?

In zt_examples_test, the doc states that we can extract the StorageError from response as:

var stgErr StorageError
if errors.As(err, &stgErr) { // We know this error is service-specific
switch stgErr.ErrorCode {
case StorageErrorCodeContainerAlreadyExists:

However, this is not valid in current implementation, since we are returning the StorageError with pointer value in handleError:

func handleError(err error) error {
var respErr *azcore.ResponseError
if errors.As(err, &respErr) {
return &InternalError{responseErrorToStorageError(respErr)}

I am also confused that, why we are declaring the StorageError struct itself as an error, but returning a pointer in handleError? Why not just declare the *StorageError as error and pass this pointer around?

  • What did you expect or want to happen?

We should update the doc (but IMO updating the error interface for StorageError is better) to:

		var stgErr *StorageError

		if errors.As(err, &stgErr) { // We know this error is service-specific
			switch stgErr.ErrorCode {
			case StorageErrorCodeContainerAlreadyExists:
  • How can we reproduce it?
internalErr := &InternalError{
  cause: &StorageError{},
}

{
    var target StorageError
    fmt.Println(errors.As(internalErr, &target))
}

{
    var target *StorageError
    fmt.Println(errors.As(internalErr, &target))
}
  • Anything we should know about your environment.
@ghost ghost added the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jan 26, 2022
@jhendrixMSFT jhendrixMSFT added the Storage Storage Service (Queues, Blobs, Files) label Jan 26, 2022
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Jan 26, 2022
@RickWinter RickWinter added Client This issue points to a problem in the data-plane of the library. bug This issue requires a change to an existing behavior in the product in order to be resolved. labels Jan 26, 2022
@jhendrixMSFT
Copy link
Member

Failed operations now return an *azcore.ResponseError, and there is a helper bloberror.HasCode() to check for specific storage error codes.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Docs Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

4 participants