apache / apache/shenyu

[BUG] HttpShenyuSdkClient wraps all failures into ShenyuException — silently disables retry (default httpclient backend)

Open
#6,853 1 comment 0 reactions 0 assignees View on GitHub
priority: medium sdk type: bug
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.