github / gh-gei

Migration CLI for GitHub to GitHub migrations
MIT License
306 stars 82 forks source link

Add Retry Logic to Download Log Command #728

Open xiaonile opened 1 year ago

xiaonile commented 1 year ago

As experienced in https://github.com/github/migration-friction/issues/752 When the CLI encounters a Bad Gateway error from dotcom the request to download logs fail with no retries.

For better customer experience/quality of life upgrade, we should add some retries as this is likely a transient networking error on the platform side.

Todo

dylan-smith commented 1 year ago

In this case it looks like it already has retry logic, but I think there might be a bug where it's not properly logging the fact that retries are happening. At least that's what it looks like to me from looking at the code real quick.

But we should be able to add a test case based on the error in that log to confirm for sure.

dylan-smith commented 1 year ago

Nope I take it back, there's a bug in our retry logic, it's only retrying on an empty string result and not on exceptions. That's definitely a bug.

We need to fix our RetryPolicy.RetryOnResult code to more closely match the example here: https://github.com/App-vNext/Polly#step-1b-optionally-specify-return-results-you-want-to-handle Specifically to do .Handle<Exception>().OrResult(...)

dpmex4527 commented 1 year ago

Closing as Bad Gateway error has been fixed on the backend when using --download-logs flag.

dylan-smith commented 1 year ago

re-opening this, as it does represent a bug in the CLI retry logic that we should still come back and fix.