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

[Storage] Re-generate autorest with version 5.0.1 #5688

Merged
merged 3 commits into from Oct 22, 2019
Merged

[Storage] Re-generate autorest with version 5.0.1 #5688

merged 3 commits into from Oct 22, 2019

Conversation

joheredi
Copy link
Member

Regenerating autorest with @microsoft.azure/autorest.typescript@5.0.1

@ramya-rao-a
Copy link
Contributor

@XiaoningLiu, @jiacfan This PR is the last step towards fixing the bug #4999

@ramya-rao-a
Copy link
Contributor

@joheredi With these changes, any error from the storage service should reach the client with an extra field details with the expected error code and so fixing the bug #4999

Can you please add some tests or update existing tests to ensure that the details field has the expected values?

@joheredi
Copy link
Member Author

@joheredi With these changes, any error from the storage service should reach the client with an extra field details with the expected error code and so fixing the bug #4999

Can you please add some tests or update existing tests to ensure that the details field has the expected values?

Thanks Ramya, I have updated Blob, Queue and File tests to check for the details property

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Oct 22, 2019

@XiaoningLiu, @jiacfan With this PR, users will now see a new field details on the error that will have the data model specified in the swagger. See 0fc9d8c

If we want to pull this error code up one level so that it can be directly on the RestError object, then the change will have to be in the convenience layer. But this PR only focuses on getting the new details property on the error when swagger specifies a data model for errors

Copy link
Member

@jiacfan jiacfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Copy link
Member

@XiaoningLiu XiaoningLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a great progress! details.errorCode deserialized from HTTP response header x-ms-error-code right?

BTW, does Response body get deserialized too with the fix? Better to take a look at the deserialization for response body. Believe swagger defines schema for error body too.

For example, https://docs.microsoft.com/en-us/rest/api/storageservices/status-and-error-codes2

<Error>  
  <Code>string-value</Code>  
  <Message>string-value</Message>  
</Error>  

@ramya-rao-a
Copy link
Contributor

details.errorCode deserialized from HTTP response header x-ms-error-code right

Yes

does Response body get deserialized too with the fix? Better to take a look at the deserialization for response body. Believe swagger defines schema for error body too

@sarangan12 Can you answer the above question?

@ramya-rao-a ramya-rao-a merged commit 27bf79b into Azure:master Oct 22, 2019
@ramya-rao-a
Copy link
Contributor

Actually, yes. The response body also gets deserialized and is accessible via error.response.parsedBody
You can also access error.response.parsedHeaders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants