mirror of
https://github.com/QwenLM/qwen-code.git
synced 2025-12-19 09:33:53 +00:00
fix: auth hang when select qwen-oauth (#684)
This commit is contained in:
@@ -712,8 +712,6 @@ async function authWithQwenDeviceFlow(
|
||||
`Polling... (attempt ${attempt + 1}/${maxAttempts})`,
|
||||
);
|
||||
|
||||
process.stdout.write('.');
|
||||
|
||||
// Wait with cancellation check every 100ms
|
||||
await new Promise<void>((resolve) => {
|
||||
const checkInterval = 100; // Check every 100ms
|
||||
|
||||
@@ -901,5 +901,37 @@ describe('SharedTokenManager', () => {
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('should properly clean up timeout when file operation completes before timeout', async () => {
|
||||
const tokenManager = SharedTokenManager.getInstance();
|
||||
tokenManager.clearCache();
|
||||
|
||||
const mockClient = {
|
||||
getCredentials: vi.fn().mockReturnValue(null),
|
||||
setCredentials: vi.fn(),
|
||||
getAccessToken: vi.fn(),
|
||||
requestDeviceAuthorization: vi.fn(),
|
||||
pollDeviceToken: vi.fn(),
|
||||
refreshAccessToken: vi.fn(),
|
||||
};
|
||||
|
||||
// Mock clearTimeout to verify it's called
|
||||
const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout');
|
||||
|
||||
// Mock file stat to resolve quickly (before timeout)
|
||||
mockFs.stat.mockResolvedValue({ mtimeMs: 12345 } as Stats);
|
||||
|
||||
// Call checkAndReloadIfNeeded which uses withTimeout internally
|
||||
const checkMethod = getPrivateProperty(
|
||||
tokenManager,
|
||||
'checkAndReloadIfNeeded',
|
||||
) as (client?: IQwenOAuth2Client) => Promise<void>;
|
||||
await checkMethod.call(tokenManager, mockClient);
|
||||
|
||||
// Verify that clearTimeout was called to clean up the timer
|
||||
expect(clearTimeoutSpy).toHaveBeenCalled();
|
||||
|
||||
clearTimeoutSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -290,6 +290,36 @@ export class SharedTokenManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Utility method to add timeout to any promise operation
|
||||
* Properly cleans up the timeout when the promise completes
|
||||
*/
|
||||
private withTimeout<T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
operationType = 'Operation',
|
||||
): Promise<T> {
|
||||
let timeoutId: NodeJS.Timeout;
|
||||
|
||||
return Promise.race([
|
||||
promise.finally(() => {
|
||||
// Clear timeout when main promise completes (success or failure)
|
||||
if (timeoutId) {
|
||||
clearTimeout(timeoutId);
|
||||
}
|
||||
}),
|
||||
new Promise<never>((_, reject) => {
|
||||
timeoutId = setTimeout(
|
||||
() =>
|
||||
reject(
|
||||
new Error(`${operationType} timed out after ${timeoutMs}ms`),
|
||||
),
|
||||
timeoutMs,
|
||||
);
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Perform the actual file check and reload operation
|
||||
* This is separated to enable proper promise-based synchronization
|
||||
@@ -303,25 +333,12 @@ export class SharedTokenManager {
|
||||
|
||||
try {
|
||||
const filePath = this.getCredentialFilePath();
|
||||
// Add timeout to file stat operation
|
||||
const withTimeout = async <T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
): Promise<T> =>
|
||||
Promise.race([
|
||||
promise,
|
||||
new Promise<never>((_, reject) =>
|
||||
setTimeout(
|
||||
() =>
|
||||
reject(
|
||||
new Error(`File operation timed out after ${timeoutMs}ms`),
|
||||
),
|
||||
timeoutMs,
|
||||
),
|
||||
),
|
||||
]);
|
||||
|
||||
const stats = await withTimeout(fs.stat(filePath), 3000);
|
||||
const stats = await this.withTimeout(
|
||||
fs.stat(filePath),
|
||||
3000,
|
||||
'File operation',
|
||||
);
|
||||
const fileModTime = stats.mtimeMs;
|
||||
|
||||
// Reload credentials if file has been modified since last cache
|
||||
@@ -451,7 +468,7 @@ export class SharedTokenManager {
|
||||
// Check if we have a refresh token before attempting refresh
|
||||
const currentCredentials = qwenClient.getCredentials();
|
||||
if (!currentCredentials.refresh_token) {
|
||||
console.debug('create a NO_REFRESH_TOKEN error');
|
||||
// console.debug('create a NO_REFRESH_TOKEN error');
|
||||
throw new TokenManagerError(
|
||||
TokenError.NO_REFRESH_TOKEN,
|
||||
'No refresh token available for token refresh',
|
||||
@@ -589,26 +606,12 @@ export class SharedTokenManager {
|
||||
const dirPath = path.dirname(filePath);
|
||||
const tempPath = `${filePath}.tmp.${randomUUID()}`;
|
||||
|
||||
// Add timeout wrapper for file operations
|
||||
const withTimeout = async <T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
): Promise<T> =>
|
||||
Promise.race([
|
||||
promise,
|
||||
new Promise<never>((_, reject) =>
|
||||
setTimeout(
|
||||
() => reject(new Error(`Operation timed out after ${timeoutMs}ms`)),
|
||||
timeoutMs,
|
||||
),
|
||||
),
|
||||
]);
|
||||
|
||||
// Create directory with restricted permissions
|
||||
try {
|
||||
await withTimeout(
|
||||
await this.withTimeout(
|
||||
fs.mkdir(dirPath, { recursive: true, mode: 0o700 }),
|
||||
5000,
|
||||
'File operation',
|
||||
);
|
||||
} catch (error) {
|
||||
throw new TokenManagerError(
|
||||
@@ -622,21 +625,30 @@ export class SharedTokenManager {
|
||||
|
||||
try {
|
||||
// Write to temporary file first with restricted permissions
|
||||
await withTimeout(
|
||||
await this.withTimeout(
|
||||
fs.writeFile(tempPath, credString, { mode: 0o600 }),
|
||||
5000,
|
||||
'File operation',
|
||||
);
|
||||
|
||||
// Atomic move to final location
|
||||
await withTimeout(fs.rename(tempPath, filePath), 5000);
|
||||
await this.withTimeout(
|
||||
fs.rename(tempPath, filePath),
|
||||
5000,
|
||||
'File operation',
|
||||
);
|
||||
|
||||
// Update cached file modification time atomically after successful write
|
||||
const stats = await withTimeout(fs.stat(filePath), 5000);
|
||||
const stats = await this.withTimeout(
|
||||
fs.stat(filePath),
|
||||
5000,
|
||||
'File operation',
|
||||
);
|
||||
this.memoryCache.fileModTime = stats.mtimeMs;
|
||||
} catch (error) {
|
||||
// Clean up temp file if it exists
|
||||
try {
|
||||
await withTimeout(fs.unlink(tempPath), 1000);
|
||||
await this.withTimeout(fs.unlink(tempPath), 1000, 'File operation');
|
||||
} catch (_cleanupError) {
|
||||
// Ignore cleanup errors - temp file might not exist
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user