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

Ignore timeout error while polling operation status from ARM #11531

Merged
merged 9 commits into from
Oct 22, 2019

Conversation

dikhakha
Copy link
Member

@dikhakha dikhakha commented Oct 9, 2019

Ignore timeout error when we poll for ARM operation status. This is to avoid failing in case poll operation request timed out due to ARM taking long time to process the request. The operation could have ended up successfully however the tasks fails because of this.

@dikhakha
Copy link
Member Author

@bishal-pdMSFT / @arjgupta / @vinodkumar3 Can you please have a look at this PR ?

}
catch (error) {
let errorString: string = (error && error.toString()) || "";
if(errorString && errorString.toLowerCase().indexOf("request timeout") >= 0 && ignoreTimeoutErrorThreshold > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is there something more to look for than just "request timeout"?

Copy link
Member Author

Choose a reason for hiding this comment

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

The error is of form "Request Timeout: ", so I think this should be good enough.

if (response.headers["retry-after"]) {
sleepDuration = parseInt(response.headers["retry-after"]);
else {
throw error;
Copy link
Contributor

@bishal-pdMSFT bishal-pdMSFT Oct 17, 2019

Choose a reason for hiding this comment

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

just throw? Not sure about javascript but in C# throw error would change the call stack

Copy link
Member Author

Choose a reason for hiding this comment

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

In javascript, you can re throw the error which will preserve the stack by doing: throw error. throw new Error(error) will lead to stack trace being lost.

@vinodkumar3
Copy link
Contributor

I think you might need to update the patch version of many tasks depending upon task common lib

@dikhakha
Copy link
Member Author

dikhakha commented Oct 21, 2019

I think you might need to update the patch version of many tasks depending upon task common lib

This functionality is not absolutely required by other tasks who are using this module. Do you think there is need to update other tasks version ? I will update for this task

@dikhakha
Copy link
Member Author

I think you might need to update the patch version of many tasks depending upon task common lib

This functionality is not absolutely required by other tasks who are using this module. Do you think there is need to update other tasks version ? I will update for this task

I see that build itself will fail if tasks consuming this common is not updated. Will update other task versions as well. Thanks

@dikhakha dikhakha merged commit 0453a78 into master Oct 22, 2019
leantk pushed a commit that referenced this pull request Dec 23, 2019
* ignore timeout error while polling for operation

* add ignore timeout error threshold

* review comments

* updating tasks version using common module
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.

3 participants