Skip to content

Commit 9119f70

Browse files
committed
Fix avatar fetch error handling for TLS certificate issues
- Wrap second doFetch() call in try-catch to prevent crashes - Add proper error logging for avatar fetch failures - Return undefined gracefully when avatar fetching fails - Improve user identification in error messages for both accounts and teams - Add test to verify the fix handles TLS errors correctly
1 parent 2a22515 commit 9119f70

2 files changed

Lines changed: 77 additions & 2 deletions

File tree

src/common/uri.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import fetch from 'cross-fetch';
1111
import * as vscode from 'vscode';
1212
import { Repository } from '../api/api';
1313
import { EXTENSION_ID } from '../constants';
14-
import { IAccount, ITeam, reviewerId } from '../github/interface';
14+
import { IAccount, isTeam, ITeam, reviewerId } from '../github/interface';
1515
import { PullRequestModel } from '../github/pullRequestModel';
1616
import { GitChangeType } from './file';
1717
import Logger from './logger';
@@ -308,7 +308,14 @@ export namespace DataUri {
308308
await doFetch();
309309
} catch (e) {
310310
// We retry once.
311-
await doFetch();
311+
try {
312+
await doFetch();
313+
} catch (retryError) {
314+
// Log the error and return undefined instead of crashing
315+
const userIdentifier = isTeam(user) ? `${user.org}/${user.slug}` : user.login || 'unknown';
316+
Logger.error(`Failed to fetch avatar after retry for user ${userIdentifier}: ${retryError}`, 'avatarCirclesAsImageDataUris');
317+
return undefined;
318+
}
312319
}
313320
}
314321
if (!innerImageContents) {

src/test/common/uri.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { default as assert } from 'assert';
7+
import * as vscode from 'vscode';
8+
import { DataUri } from '../../common/uri';
9+
import { IAccount, AccountType } from '../../github/interface';
10+
11+
describe('DataUri.avatarCirclesAsImageDataUris', () => {
12+
let mockContext: vscode.ExtensionContext;
13+
let originalFetch: any;
14+
15+
beforeEach(() => {
16+
// Save original fetch
17+
originalFetch = global.fetch;
18+
19+
// Create a mock extension context
20+
mockContext = {
21+
globalStorageUri: vscode.Uri.file('/tmp/test-storage'),
22+
} as any as vscode.ExtensionContext;
23+
});
24+
25+
afterEach(() => {
26+
// Restore original fetch
27+
global.fetch = originalFetch;
28+
});
29+
30+
it('should handle TLS certificate errors gracefully without crashing', async () => {
31+
// Mock user with avatar URL
32+
const testUser: IAccount = {
33+
login: 'testuser',
34+
id: '123',
35+
avatarUrl: 'https://avatars.githubusercontent.com/u/123?v=4',
36+
url: 'https://github.com/testuser',
37+
accountType: AccountType.User
38+
};
39+
40+
// Mock fetch to simulate TLS certificate error
41+
global.fetch = () => Promise.reject(
42+
new Error('request to https://avatars.githubusercontent.com/u/123?v=4 failed, reason: self signed certificate in certificate chain')
43+
);
44+
45+
// Mock vscode.workspace.fs to simulate cache miss
46+
const originalReadFile = vscode.workspace.fs.readFile;
47+
const originalWriteFile = vscode.workspace.fs.writeFile;
48+
const originalCreateDirectory = vscode.workspace.fs.createDirectory;
49+
50+
vscode.workspace.fs.readFile = () => Promise.reject(new Error('Cache miss'));
51+
vscode.workspace.fs.writeFile = () => Promise.resolve();
52+
vscode.workspace.fs.createDirectory = () => Promise.resolve();
53+
54+
try {
55+
// This should not throw an error even with TLS issues
56+
const results = await DataUri.avatarCirclesAsImageDataUris(mockContext, [testUser], 20, 20);
57+
58+
// Should return array with undefined for failed fetches
59+
assert.strictEqual(results.length, 1);
60+
assert.strictEqual(results[0], undefined);
61+
} finally {
62+
// Restore original functions
63+
vscode.workspace.fs.readFile = originalReadFile;
64+
vscode.workspace.fs.writeFile = originalWriteFile;
65+
vscode.workspace.fs.createDirectory = originalCreateDirectory;
66+
}
67+
});
68+
});

0 commit comments

Comments
 (0)