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

Delay retrying to send telemetry if an exception is hit #420

Merged
merged 2 commits into from
Sep 15, 2017

Conversation

leantk
Copy link
Contributor

@leantk leantk commented Sep 7, 2017

I originally made changes to delay retries on specific exceptions but users are hitting this for more reasons such as working offline, working through a proxy, etc. They see >500 requests in less than a minute in these cases which have taken down their proxy. I don't know any reason why there shouldn't be backoff if there is an exception in general instead of constantly retrying. I made the change to delay no matter the exception. I tested on my personal machine and I stopped seeing the constant retries every second.

@msftclas
Copy link

msftclas commented Sep 7, 2017

@leantk,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@Dmitry-Matveev
Copy link
Member

Discussed offline, approving PR but waiting for this one to complete first (may require small merge commit).

@Dmitry-Matveev
Copy link
Member

This PR covers the case for the IllegalStateException as well, so merging this one first. We'll continue discussion with @andresol in #405 .
@leantk, could you please add changelog.md entry for this change?

@leantk
Copy link
Contributor Author

leantk commented Sep 15, 2017

@Dmitry-Matveev I made the changelog update. If you need me to do anything else let me know.

@dhaval24 dhaval24 merged commit cbdef0c into microsoft:master Sep 15, 2017
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.

4 participants