Remove `RequestLog` dependency in `RetryingClient`
Motivation:
Related: #5708, #4834
If a decorator directly returns a response, the RequestLog for the request won't be complete. Because the RequestLog is automatically completed when it starts to be written to the wire by AbstractRequestHandler.
In the case of RetryingClient, RequestLog's events are used to get exceptions to apply RetryRule. If the RequestLog events aren't complete, RetryingClient will fall into a deadlock.
Typically, RequestLog is automatically completed so it is difficult for the user to recognize that RequestLog must be completed. Consequently, it is easy to miss.
I propose two changes in this PR to solve the problem.
-
If
RequestLog.requestFirstBytesTransferredTimeNanosisn't logged when a response completes, we can assume that the response is returned by a decorator. A hook is added to a response completion event to complete the log manually for that case. -
RetryingClientis refactored not to retry onRequestLogbut to useSplitHttpResponseand subscribe to headers. An exception when headers are received is used to applyRetryRule.
Modifications:
- Add a hook that completes
RequestLogif it is written to wire toClientUtil - Split
handleResponseintohandleAggregatedResponseandhandleStreamingResponseinRetryingClient- Remove
RequestLogusage inhandleStreamingResponseand useSplitHttpRequestto check if an exception is raised in the response.
- Remove
Result:
- Fixed deadlock that could occur in
RetryingClientdue toRequestLognot being completed.
🔍 Build Scan® (commit: 8fa148322fdeb293ab366a894eedbfddad1d8c85)
| Job name | Status | Build Scan® |
|---|---|---|
| build-ubicloud-standard-16-jdk-8 | ✅ | https://ge.armeria.dev/s/sd5ajsnkj57xo |
| build-ubicloud-standard-16-jdk-21-snapshot-blockhound | ❌ (failure) | https://ge.armeria.dev/s/hjoxju3hrnqfe |
| build-ubicloud-standard-16-jdk-17-min-java-17-coverage | ❌ (failure) | https://ge.armeria.dev/s/buok3kfrvfyxs |
| build-ubicloud-standard-16-jdk-17-min-java-11 | ✅ | https://ge.armeria.dev/s/76avtz3s5lsoi |
| build-ubicloud-standard-16-jdk-17-leak | ✅ | https://ge.armeria.dev/s/fxzvtiirv2huy |
| build-ubicloud-standard-16-jdk-11 | ✅ | https://ge.armeria.dev/s/euzvr2c22nb3m |
| build-macos-latest-jdk-21 | ✅ | https://ge.armeria.dev/s/a76grpdrbryw4 |
This is ready for review.