fix(core): handle empty OAuth refresh response body (#3123)
Some checks are pending
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run

* fix(core): handle empty OAuth refresh response body

When Qwen's OAuth server returns 200 with an empty body (e.g., stale
refresh token), response.json() throws 'Expecting value: line 1 column 1
(char 0)' instead of a usable error message. This forces users to
re-authenticate with no indication of what went wrong.

Fix: read response.text() first, then JSON.parse with a try/catch that
clears credentials and provides a clear error message.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): address review feedback on OAuth refresh error handling

- Don't clear credentials on malformed 200 responses (treat as retryable)
- Clear credentials on explicit 400/401 auth-invalid responses
- Add text() to all refresh-path test mocks
- Add regression tests for malformed 200 and 401 responses

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
John London 2026-04-11 22:08:23 -05:00 committed by GitHub
parent 19f2d292f9
commit fea61e1788
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 107 additions and 3 deletions

View file

@ -367,6 +367,13 @@ describe('QwenOAuth2Client', () => {
it('should successfully refresh access token', async () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
access_token: 'new-access-token',
token_type: 'Bearer',
expires_in: 3600,
resource_url: 'https://new-endpoint.com',
}),
json: async () => ({
access_token: 'new-access-token',
token_type: 'Bearer',
@ -394,6 +401,11 @@ describe('QwenOAuth2Client', () => {
it('should handle refresh error', async () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
error: 'INVALID_GRANT',
error_description: 'The refresh token is invalid',
}),
json: async () => ({
error: 'INVALID_GRANT',
error_description: 'The refresh token is invalid',
@ -413,6 +425,13 @@ describe('QwenOAuth2Client', () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
access_token: 'new-access-token',
token_type: 'Bearer',
expires_in: 3600,
resource_url: 'https://new-endpoint.com',
}),
json: async () => ({
access_token: 'new-access-token',
token_type: 'Bearer',
@ -450,6 +469,14 @@ describe('QwenOAuth2Client', () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
access_token: 'new-access-token',
token_type: 'Bearer',
expires_in: 3600,
refresh_token: 'new-refresh-token',
resource_url: 'https://new-endpoint.com',
}),
json: async () => ({
access_token: 'new-access-token',
token_type: 'Bearer',
@ -715,6 +742,49 @@ describe('QwenOAuth2Client', () => {
'Token refresh failed: 500 Internal Server Error',
);
});
it('should NOT clear credentials on malformed 200 response (e.g. proxy HTML)', async () => {
const { CredentialsClearRequiredError } = await import('./qwenOAuth2.js');
const mockResponse = {
ok: true,
status: 200,
text: async () => '<html><body>Proxy Error</body></html>',
};
vi.mocked(global.fetch).mockResolvedValue(mockResponse as Response);
// Should throw a retryable Error, NOT CredentialsClearRequiredError
// (CredentialsClearRequiredError implies credentials were cleared)
await expect(client.refreshAccessToken()).rejects.toBeInstanceOf(Error);
await expect(client.refreshAccessToken()).rejects.not.toBeInstanceOf(
CredentialsClearRequiredError,
);
await expect(client.refreshAccessToken()).rejects.toThrow(
'Qwen OAuth refresh returned invalid JSON:',
);
});
it('should clear credentials and throw CredentialsClearRequiredError on 401 response', async () => {
const { CredentialsClearRequiredError } = await import('./qwenOAuth2.js');
const mockResponse = {
ok: false,
status: 401,
statusText: 'Unauthorized',
text: async () => 'Unauthorized',
};
vi.mocked(global.fetch).mockResolvedValue(mockResponse as Response);
await expect(client.refreshAccessToken()).rejects.toBeInstanceOf(
CredentialsClearRequiredError,
);
await expect(client.refreshAccessToken()).rejects.toThrow(
"Refresh token expired or invalid. Please use '/auth' to re-authenticate.",
);
});
});
describe('credentials management', () => {
@ -1620,6 +1690,12 @@ describe('Credential Caching Functions', () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
access_token: 'new-token',
token_type: 'Bearer',
expires_in: 3600,
}),
json: async () => ({
access_token: 'new-token',
token_type: 'Bearer',
@ -1965,6 +2041,13 @@ describe('Enhanced Error Handling and Edge Cases', () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
access_token: 'new-access-token',
token_type: 'Bearer',
expires_in: 3600,
// No refresh_token in response
}),
json: async () => ({
access_token: 'new-access-token',
token_type: 'Bearer',
@ -1988,6 +2071,13 @@ describe('Enhanced Error Handling and Edge Cases', () => {
const mockResponse = {
ok: true,
text: async () =>
JSON.stringify({
access_token: 'new-access-token',
token_type: 'Bearer',
expires_in: 3600,
resource_url: 'https://new-resource-url.com',
}),
json: async () => ({
access_token: 'new-access-token',
token_type: 'Bearer',

View file

@ -420,8 +420,8 @@ export class QwenOAuth2Client implements IQwenOAuth2Client {
if (!response.ok) {
const errorData = await response.text();
// Handle 400 errors which might indicate refresh token expiry
if (response.status === 400) {
// Handle 400/401 errors which indicate refresh token expiry or invalidity
if (response.status === 400 || response.status === 401) {
await clearQwenCredentials();
throw new CredentialsClearRequiredError(
"Refresh token expired or invalid. Please use '/auth' to re-authenticate.",
@ -433,7 +433,21 @@ export class QwenOAuth2Client implements IQwenOAuth2Client {
);
}
const responseData = (await response.json()) as TokenRefreshResponse;
let responseText: string;
try {
responseText = await response.text();
} catch {
responseText = '';
}
let responseData: TokenRefreshResponse;
try {
responseData = JSON.parse(responseText) as TokenRefreshResponse;
} catch {
throw new Error(
`Qwen OAuth refresh returned invalid JSON: ${responseText || '(empty response body)'}`,
);
}
// Check if the response indicates success
if (isErrorResponse(responseData)) {