fix: Enabled automatic HTTP retries for FirebaseProjectManagement - #356
Conversation
|
|
||
| // Verify retry support | ||
| HttpUnsuccessfulResponseHandler retryHandler = request.getUnsuccessfulResponseHandler(); | ||
| assertTrue(retryHandler instanceof RetryHandlerDecorator); |
There was a problem hiding this comment.
At first I kind of want to say we should bake this assertion into the API infrastructure itself instead of directly verifying in a unit test that no one touched the code. But then the OO design I came up with to facilitate this turns out to be impossible due to various classes being final. I was hoping that we can write a subclass of HttpRequest called RetryingHttpRequest that declares at compile time that getUnsuccessfulResponseHandler will return RetryHandlerDecorator or a suitable base class, but alas, HttpRequest is final. (And so is HttpRequestFactory, which is the other class I wanted to subclass.)
Without the above OO structure, the only other way to structure the test to be more behavioural would be to treat the retrying mechanism completely like a black box, and empirically measure retry intervals and retry counts after forcing an unsuccessful response. This is going to be painfully flaky, so I think what you have is probably for the best.
weixifan
left a comment
There was a problem hiding this comment.
Overall LGTM. The only comment I left is just an interesting observation.
Enables automatic HTTP retries for the
FirebaseProjectManagementAPI. We disable retries for unit tests for faster execution of tests.RELEASE NOTE: Project management APIs in the
FirebaseProjectManagementclass now automatically retries operations that fail due to retry-eligible HTTP errors.