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

New Generic Poller doesn't correctly handle case where Poll request succeeds, but with failed state #18581

Closed
matthchr opened this issue Jul 12, 2022 · 2 comments
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@matthchr
Copy link
Member

Bug Report

I was specifically testing with github.com/Azure/azure-sdk-for-go/sdk/azcore v1.1.1-0.20220609210213-3e53bd32358a but I think any current version of Poller[T] will have this issue.

This shows up most obviously as a change in behavior of the Poll function of the Poller[T]. In the old, non-generic implementation, a Poll that recieved a valid HTTP response (HTTP status code 200), but whose contents contained a Failed state with an error, would classify that as an error in Golang and return the error to the caller of Poll. You can see this here, the snippet from Poll is:

	if Failed(l.lro.Status()) {
		l.err = shared.NewResponseError(resp)
		l.resp = nil
		return nil, l.err
	}

Now though, if the Poller[T] has a successful response such as:

{"name":"6ec3dd1f-ebf8-4a8c-a8dc-9c5f8fd37358","status":"Failed","startTime":"2022-07-12T20:45:59.39Z","error":{"code":"InternalServerError","message":"An unexpected error occured while processing the request. Tracking ID: 'a675d3d1-a6fd-4938-bafe-e491662d2966'"}}

This is not classified as an error and Poll doesn't return an error. As far as I can tell in the current Poller[T] impl, there is no equivalent "if failed" check. See the code.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 12, 2022
@jhendrixMSFT
Copy link
Member

Per offline discussion, the pattern is now as follows.

poller := BeginLRO()
for !poller.Done() {
    resp, err := poller.Poll()
    // handle above
}

result, err := poller.Result()
// here's where you discover if the LRO succeeded or failed

@matthchr
Copy link
Member Author

Thanks @jhendrixMSFT!

@RickWinter RickWinter added question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Client This issue points to a problem in the data-plane of the library. Azure.Core labels Oct 6, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Oct 6, 2022
@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
Azure.Core Client This issue points to a problem in the data-plane of the library. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

3 participants