apache / apache/shenyu

[BUG] HTTP sync token refresh can stop after a login response parsing error

Open
#6,504 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
8.8k
Forks
3.1k
Avg merge
7d 1h
Merged PRs (30d)
85

Description

### Search before asking

- [x] I had searched in the [issues](https://github.com/apache/shenyu/issues) and found no similar issues.

### Apache ShenYu Component

shenyu-sync-data-http

### What happened

`AccessTokenManager` starts a periodic token refresh task with `scheduleWithFixedDelay`:

```java
private void start(final List servers) {
this.login(servers);
this.executorService.scheduleWithFixedDelay(() -> this.login(servers), 5000, 5000, TimeUnit.MILLISECONDS);
}
```

But `doLogin(...)` only catches `IOException`:

```java
try (Response response = this.okHttpClient.newCall(request).execute()) {
...
Map resultMap = GsonUtils.getInstance().convertToMap(result);
...
Map tokenMap = GsonUtils.getInstance().convertToMap(tokenJson);
this.accessToken = (String) tokenMap.get(Constants.ADMIN_RESULT_TOKEN);
this.tokenExpiredTime = (long) tokenMap.get(Constants.ADMIN_RESULT_EXPIRED_TIME);
this.tokenRefreshWindow = this.tokenExpiredTime / 10;
return true;
} catch (IOException e) {
LOG.error("get token from server : [{}] error", server, e);
return false;
}
```

Runtime exceptions from response parsing or field conversion are not caught. For example, malformed JSON from a temporary admin/proxy error page can throw a JSON parsing exception. Also `expiredTime` is read through `Map` and cast directly with `(long)`, so a non-`Long` numeric representation can throw `ClassCastException`.

Because the exception escapes the `scheduleWithFixedDelay` task, `ScheduledThreadPoolExecutor` suppresses all future refresh executions. Once the current token expires, HTTP sync can no longer authenticate to admin until the gateway is restarted.

### Expected behavior

One bad login response or conversion error should be logged and treated like a failed login attempt. The scheduled refresh task should continue retrying later servers/later intervals. The login response should also be parsed into a typed DTO or converted through `Number.longValue()` rather than directly casting `Object` to `long`.

### How to reproduce

1. Enable HTTP data sync with admin credentials.
2. Make the admin login endpoint temporarily return a malformed JSON body or a login `data.expiredTime` value that is not represented as `Long` in `Map`.
3. Let the refresh task run.
4. `doLogin(...)` throws a runtime exception that is not caught by the `IOException` handler.
5. The scheduled refresh task stops permanently, so token refresh never retries.

### Debug logs

_No response_

### Environment

Current `master` branch.

### Are you willing to submit a PR?

- [ ] Yes I am willing to submit a PR!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with `AccessTokenManager.start(...)` and `doLogin(...)` in the `shenyu-sync-data-http` component, then inspect existing tests for token login and refresh behavior. Check how parsing and conversion failures are handled and whether scheduled refresh continues after a failed attempt. Done when a malformed response or invalid `expiredTime` is treated as a failed login and later scheduled refreshes still run.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
67/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.