[BUG] HttpShenyuSdkClient wraps all failures into ShenyuException — silently disables retry (default httpclient backend)
- Dominant language
- Java
- Stars
- 8.8k
- Forks
- 3.1k
- Avg merge
- 7d 1h
- Merged PRs (30d)
- 85
Description
## Description
`HttpShenyuSdkClient.doRequest` wraps every failure from `execute.get()` / `EntityUtils.toString` in `catch (Exception e) { throw new ShenyuException(e); }` (a `RuntimeException`, not `IOException`). `AbstractShenyuSdkClient.execute0` only catches `IOException` (line 119) to convert it into a `RetryableException`; `execute` only catches `RetryableException` (line 108). A plain `ShenyuException` is neither an `IOException` nor a `RetryableException` (`RetryableException extends ShenyuException`, not the reverse), so it propagates straight out of `execute` with no retry attempt and no `RetryableException` wrapping. The OkHttp backend throws `IOException` directly (caught → `RetryableException`), so retry works there; the default `clientType=httpclient` backend (`ShenyuSdkAutoConfiguration:87`) does not. As a side effect, `InterruptedException` from `execute.get()` is caught-and-wrapped without `Thread.currentThread().interrupt()`, also losing interrupt status.
## Location
- `shenyu-sdk/shenyu-sdk-httpclient/src/main/java/org/apache/shenyu/sdk/httpclient/HttpShenyuSdkClient.java:165-173`
- `shenyu-sdk/shenyu-sdk-core/src/main/java/org/apache/shenyu/sdk/core/client/AbstractShenyuSdkClient.java:108,114-124`
## Impact
When a user sets `shenyu.sdk.props.retry.enable=true` with the default httpclient backend, the configured `Retryer.DefaultRetry` is constructed but never invoked — every transport failure propagates immediately with zero retries, silently violating the retry contract. Callers that catch `RetryableException` to distinguish retryable vs fatal errors also see a different exception type than the OkHttp path.
## Suggested fix
In `HttpShenyuSdkClient.doRequest`, let `IOException` propagate (or unwrap `ExecutionException`/`InterruptedException` and rethrow `IOException` for transport failures so `execute0` can wrap it as `RetryableException`); re-assert interrupt status before rethrowing on `InterruptedException`.
## Related existing
None — distinct from #6583/#6584 (gateway-side Exponential/Fixed retry + outer timeout) and #6636/HC-EXP-01 (plugin `ExponentialRetryBackoffStrategy`, different module). No baseline entry covers `shenyu-sdk` `ShenyuSdkClient` retry.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with HttpShenyuSdkClient.java lines 165-173 and trace how execute.get() and EntityUtils.toString failures reach AbstractShenyuSdkClient.java lines 108 and 114-124. Compare the httpclient and OkHttp exception paths, then verify that default-httpclient transport failures use the existing retry path, preserve interrupt status, and expose the expected exception type.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100