Adds avatar retries

Moves avatar cache to global storage
Improves avatar cache updates/resets
Improves contributor avatar fetch update

src/annotations/gutterBlameAnnotationProvider.ts

@@ -215,7 +215,7 @@ export class GutterBlameAnnotationProvider extends BlameAnnotationProviderBase {
 				height: '16px',
 				width: '16px',
 				textDecoration: `none;position:absolute;top:1px;left:5px;background:url(${(
-					await commit.getAvatarUri({ fallback: gravatarDefault })
+					await commit.getAvatarUri({ defaultStyle: gravatarDefault })
 				).toString()});background-size:16px 16px`,
 			};
 			map.set(commit.email!, avatarDecoration);

src/avatars.ts

@@ -2,23 +2,51 @@
 import * as fs from 'fs';
 import { EventEmitter, Uri } from 'vscode';
 import { GravatarDefaultStyle } from './config';
-import { WorkspaceState } from './constants';
+import { GlobalState } from './constants';
 import { Container } from './container';
 import { GitRevisionReference } from './git/git';
-import { Functions, Strings } from './system';
+import { Functions, Iterables, Strings } from './system';
+import { MillisecondsPerDay, MillisecondsPerHour, MillisecondsPerMinute } from './system/date';
 import { ContactPresenceStatus } from './vsls/vsls';
 
-// TODO@eamodio Use timestamp
-// TODO@eamodio Clear avatar cache on remote / provider connection change
+const _onDidFetchAvatar = new EventEmitter<{ email: string }>();
+_onDidFetchAvatar.event(
+	Functions.debounce(() => {
+		const avatars =
+			avatarCache != null
+				? [
+						...Iterables.filterMap(avatarCache, ([key, avatar]) =>
+							avatar.uri != null
+								? [
+										key,
+										{
+											uri: avatar.uri.toString(),
+											timestamp: avatar.timestamp,
+										},
+								  ]
+								: undefined,
+						),
+				  ]
+				: undefined;
+		void Container.context.globalState.update(GlobalState.Avatars, avatars);
+	}, 1000),
+);
 
-interface Avatar<T = Uri> {
-	uri?: T | null;
-	fallback: T;
+export namespace Avatars {
+	export const onDidFetch = _onDidFetchAvatar.event;
+}
+
+interface Avatar {
+	uri?: Uri;
+	fallback?: Uri;
 	timestamp: number;
-	// TODO@eamodio Add a fail count, to avoid failing on a single failure
+	retries: number;
 }
 
-type SerializedAvatar = Avatar<string>;
+interface SerializedAvatar {
+	uri: string;
+	timestamp: number;
+}
 
 let avatarCache: Map<string, Avatar> | undefined;
 const avatarQueue = new Map<string, Promise<Uri>>();
@@ -29,99 +57,107 @@ const presenceCache = new Map();
 
 const gitHubNoReplyAddressRegex = /^(?:(?<userId>\d+)\+)?(?<userName>[a-zA-Z\d-]{1,39})@users\.noreply\.github\.com$/;
 
-const _onDidFetchAvatar = new EventEmitter<{ email: string }>();
-export const onDidFetchAvatar = _onDidFetchAvatar.event;
-
-onDidFetchAvatar(
-	Functions.debounce(() => {
-		void Container.context.workspaceState.update(
-			WorkspaceState.Avatars,
-			avatarCache == null
-				? undefined
-				: [...avatarCache.entries()].map<[string, SerializedAvatar]>(([key, value]) => [
-						key,
-						{
-							uri: value.uri != null ? value.uri.toString() : value.uri,
-							fallback: value.fallback.toString(),
-							timestamp: value.timestamp,
-						},
-				  ]),
-		);
-	}, 1000),
-);
-
-export function clearAvatarCache() {
-	avatarCache?.clear();
-	avatarQueue.clear();
-	void Container.context.workspaceState.update(WorkspaceState.Avatars, undefined);
-}
-
-function ensureAvatarCache(cache: Map<string, Avatar> | undefined): asserts cache is Map<string, Avatar> {
-	if (cache == null) {
-		const avatars: [string, Avatar][] | undefined = Container.context.workspaceState
-			.get<[string, SerializedAvatar][]>(WorkspaceState.Avatars)
-			?.map<[string, Avatar]>(([key, value]) => [
-				key,
-				{
-					uri: value.uri != null ? Uri.parse(value.uri) : value.uri,
-					fallback: Uri.parse(value.fallback),
-					timestamp: value.timestamp,
-				},
-			]);
-		avatarCache = new Map<string, Avatar>(avatars);
-	}
-}
+const retryDecay = [
+	MillisecondsPerDay * 7, // First item is cache expiration (since retries will be 0)
+	MillisecondsPerMinute,
+	MillisecondsPerMinute * 5,
+	MillisecondsPerMinute * 10,
+	MillisecondsPerHour,
+	MillisecondsPerDay,
+	MillisecondsPerDay * 7,
+];
 
 export function getAvatarUri(
 	email: string | undefined,
 	repoPathOrCommit: string | GitRevisionReference | undefined,
-	{ fallback, size = 16 }: { fallback?: GravatarDefaultStyle; size?: number } = {},
+	{ defaultStyle, size = 16 }: { defaultStyle?: GravatarDefaultStyle; size?: number } = {},
 ): Uri | Promise<Uri> {
 	ensureAvatarCache(avatarCache);
 
 	if (email == null || email.length === 0) {
-		const key = `${missingGravatarHash}:${size}`;
-
-		let avatar = avatarCache.get(key);
-		if (avatar == null) {
-			avatar = {
-				fallback: Uri.parse(
-					`https://www.gravatar.com/avatar/${missingGravatarHash}.jpg?s=${size}&d=${fallback}`,
-				),
-				timestamp: Date.now(),
-			};
-			avatarCache.set(key, avatar);
-		}
-
-		return avatar.uri ?? avatar.fallback;
+		const avatar = createOrUpdateAvatar(
+			`${missingGravatarHash}:${size}`,
+			undefined,
+			missingGravatarHash,
+			size,
+			defaultStyle,
+		);
+		return avatar.uri ?? avatar.fallback!;
 	}
 
 	const hash = Strings.md5(email.trim().toLowerCase(), 'hex');
 	const key = `${hash}:${size}`;
 
-	let avatar = avatarCache.get(key);
-	if (avatar == null) {
-		avatar = {
-			uri: getAvatarUriFromGitHubNoReplyAddress(email, size),
-			fallback: Uri.parse(`https://www.gravatar.com/avatar/${hash}.jpg?s=${size}&d=${fallback}`),
-			timestamp: Date.now(),
-		};
-		avatarCache.set(key, avatar);
-	}
-
+	const avatar = createOrUpdateAvatar(
+		key,
+		getAvatarUriFromGitHubNoReplyAddress(email, size),
+		hash,
+		size,
+		defaultStyle,
+	);
 	if (avatar.uri != null) return avatar.uri;
 
 	let query = avatarQueue.get(key);
-	if (query == null && avatar.uri === undefined && repoPathOrCommit != null) {
-		query = getAvatarUriFromRemoteProvider(key, email, repoPathOrCommit, avatar.fallback, { size: size }).then(
-			uri => uri ?? avatar!.uri ?? avatar!.fallback,
+	if (query == null && repoPathOrCommit != null && hasAvatarExpired(avatar)) {
+		query = getAvatarUriFromRemoteProvider(avatar, key, email, repoPathOrCommit, { size: size }).then(
+			uri => uri ?? avatar.uri ?? avatar.fallback!,
 		);
 		avatarQueue.set(key, query);
 	}
 
 	if (query != null) return query;
 
-	return avatar.uri ?? avatar.fallback;
+	return avatar.uri ?? avatar.fallback!;
+}
+
+function createOrUpdateAvatar(
+	key: string,
+	uri: Uri | undefined,
+	hash: string,
+	size: number,
+	defaultStyle?: GravatarDefaultStyle,
+): Avatar {
+	let avatar = avatarCache!.get(key);
+	if (avatar == null) {
+		avatar = {
+			uri: uri,
+			fallback: getAvatarUriFromGravatar(hash, size, defaultStyle),
+			timestamp: 0,
+			retries: 0,
+		};
+		avatarCache!.set(key, avatar);
+	} else if (avatar.fallback == null) {
+		avatar.fallback = getAvatarUriFromGravatar(hash, size, defaultStyle);
+	}
+	return avatar;
+}
+
+function ensureAvatarCache(cache: Map<string, Avatar> | undefined): asserts cache is Map<string, Avatar> {
+	if (cache == null) {
+		const avatars: [string, Avatar][] | undefined = Container.context.globalState
+			.get<[string, SerializedAvatar][]>(GlobalState.Avatars)
+			?.map<[string, Avatar]>(([key, avatar]) => [
+				key,
+				{
+					uri: Uri.parse(avatar.uri),
+					timestamp: avatar.timestamp,
+					retries: 0,
+				},
+			]);
+		avatarCache = new Map<string, Avatar>(avatars);
+	}
+}
+
+function hasAvatarExpired(avatar: Avatar) {
+	return Date.now() >= avatar.timestamp + retryDecay[Math.min(avatar.retries, retryDecay.length - 1)];
+}
+
+function getAvatarUriFromGravatar(
+	hash: string,
+	size: number,
+	defaultStyle: GravatarDefaultStyle = GravatarDefaultStyle.Robot,
+): Uri {
+	return Uri.parse(`https://www.gravatar.com/avatar/${hash}.jpg?s=${size}&d=${defaultStyle}`);
 }
 
 function getAvatarUriFromGitHubNoReplyAddress(email: string, size: number = 16): Uri | undefined {
@@ -133,10 +169,10 @@ function getAvatarUriFromGitHubNoReplyAddress(email: string, size: number = 16):
 }
 
 async function getAvatarUriFromRemoteProvider(
+	avatar: Avatar,
 	key: string,
 	email: string,
 	repoPathOrCommit: string | GitRevisionReference,
-	fallback: Uri,
 	{ size = 16 }: { size?: number } = {},
 ) {
 	ensureAvatarCache(avatarCache);
@@ -152,26 +188,29 @@ async function getAvatarUriFromRemoteProvider(
 			account = await remote?.provider.getAccountForCommit(repoPathOrCommit.ref, { avatarSize: size });
 		}
 		if (account == null) {
-			avatarCache.set(key, { uri: null, fallback: fallback, timestamp: Date.now() });
+			// If we have no account assume that won't change (without a reset), so set the timestamp to "never expire"
+			avatar.uri = undefined;
+			avatar.timestamp = Number.MAX_SAFE_INTEGER;
+			avatar.retries = 0;
 
 			return undefined;
 		}
 
-		const uri = Uri.parse(account.avatarUrl);
-		avatarCache.set(key, { uri: uri, fallback: fallback, timestamp: Date.now() });
+		avatar.uri = Uri.parse(account.avatarUrl);
+		avatar.timestamp = Date.now();
+		avatar.retries = 0;
+
 		if (account.email != null && Strings.equalsIgnoreCase(email, account.email)) {
-			avatarCache.set(`${Strings.md5(account.email.trim().toLowerCase(), 'hex')}:${size}`, {
-				uri: uri,
-				fallback: fallback,
-				timestamp: Date.now(),
-			});
+			avatarCache.set(`${Strings.md5(account.email.trim().toLowerCase(), 'hex')}:${size}`, { ...avatar });
 		}
 
 		_onDidFetchAvatar.fire({ email: email });
 
-		return uri;
+		return avatar.uri;
 	} catch {
-		avatarCache.set(key, { uri: null, fallback: fallback, timestamp: Date.now() });
+		avatar.uri = undefined;
+		avatar.timestamp = Date.now();
+		avatar.retries++;
 
 		return undefined;
 	} finally {
@@ -192,3 +231,29 @@ export function getPresenceDataUri(status: ContactPresenceStatus) {
 
 	return dataUri;
 }
+
+export function resetAvatarCache(reset: 'all' | 'failed' | 'fallback') {
+	switch (reset) {
+		case 'all':
+			void Container.context.globalState.update(GlobalState.Avatars, undefined);
+			avatarCache?.clear();
+			avatarQueue.clear();
+			break;
+
+		case 'failed':
+			for (const avatar of avatarCache?.values() ?? []) {
+				// Reset failed requests
+				if (avatar.uri == null) {
+					avatar.timestamp = 0;
+					avatar.retries = 0;
+				}
+			}
+			break;
+
+		case 'fallback':
+			for (const avatar of avatarCache?.values() ?? []) {
+				avatar.fallback = undefined;
+			}
+			break;
+	}
+}

src/constants.ts

@@ -132,7 +132,8 @@ export enum GlyphChars {
 }
 
 export enum GlobalState {
-	GitLensVersion = 'gitlensVersion',
+	Avatars = 'gitlens:avatars',
+	Version = 'gitlensVersion',
 }
 
 export const ImageMimetypes: Record<string, string> = {
@@ -182,7 +183,6 @@ export interface StarredRepositories {
 }
 
 export enum WorkspaceState {
-	Avatars = 'gitlens:avatars',
 	BranchComparisons = 'gitlens:branch:comparisons',
 	DefaultRemote = 'gitlens:remote:default',
 	PinnedComparisons = 'gitlens:pinned:comparisons',

src/container.ts

@@ -3,7 +3,7 @@ import { commands, ConfigurationChangeEvent, ConfigurationScope, ExtensionContex
 import { Autolinks } from './annotations/autolinks';
 import { FileAnnotationController } from './annotations/fileAnnotationController';
 import { LineAnnotationController } from './annotations/lineAnnotationController';
-import { clearAvatarCache } from './avatars';
+import { resetAvatarCache } from './avatars';
 import { GitCodeLensController } from './codelens/codeLensController';
 import { Commands, ToggleFileAnnotationCommandArgs } from './commands';
 import { AnnotationsToggleMode, Config, configuration, ConfigurationWillChangeEvent } from './configuration';
@@ -108,7 +108,7 @@ export class Container {
 		}
 
 		if (configuration.changed(e.change, 'defaultGravatarsStyle')) {
-			clearAvatarCache();
+			resetAvatarCache('fallback');
 		}
 
 		if (configuration.changed(e.change, 'mode') || configuration.changed(e.change, 'modes')) {

src/extension.ts

@@ -55,7 +55,7 @@ export async function activate(context: ExtensionContext) {
 
 	const cfg = configuration.get();
 
-	const previousVersion = context.globalState.get<string>(GlobalState.GitLensVersion);
+	const previousVersion = context.globalState.get<string>(GlobalState.Version);
 	await migrateSettings(context, previousVersion);
 
 	try {
@@ -83,7 +83,7 @@ export async function activate(context: ExtensionContext) {
 	notifyOnUnsupportedGitVersion(gitVersion);
 	void showWelcomeOrWhatsNew(gitlensVersion, previousVersion);
 
-	void context.globalState.update(GlobalState.GitLensVersion, gitlensVersion);
+	void context.globalState.update(GlobalState.Version, gitlensVersion);
 
 	Logger.log(
 		`GitLens (v${gitlensVersion}${cfg.mode.active ? `, mode: ${cfg.mode.active}` : ''}) activated ${

src/git/formatters/commitFormatter.ts

@@ -223,7 +223,7 @@ export class CommitFormatter extends Formatter {
 	private async _getAvatarMarkdown(title: string) {
 		const size = Container.config.hovers.avatarSize;
 		const avatarPromise = this._item.getAvatarUri({
-			fallback: Container.config.defaultGravatarsStyle,
+			defaultStyle: Container.config.defaultGravatarsStyle,
 			size: size,
 		});
 		return this._padOrTruncate(

src/git/gitService.ts

@@ -19,6 +19,7 @@ import {
 	WorkspaceFoldersChangeEvent,
 } from 'vscode';
 import { API as BuiltInGitApi, Repository as BuiltInGitRepository, GitExtension } from '../@types/git';
+import { resetAvatarCache } from '../avatars';
 import { BranchSorting, configuration, TagSorting } from '../configuration';
 import { CommandContext, DocumentSchemes, GlyphChars, setCommandContext } from '../constants';
 import { Container } from '../container';
@@ -140,7 +141,12 @@ export class GitService implements Disposable {
 			window.onDidChangeWindowState(this.onWindowStateChanged, this),
 			workspace.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this),
 			configuration.onDidChange(this.onConfigurationChanged, this),
-			Authentication.onDidChange(() => this._remotesWithApiProviderCache.clear()),
+			Authentication.onDidChange(e => {
+				if (e.reason === 'connected') {
+					resetAvatarCache('failed');
+				}
+				this._remotesWithApiProviderCache.clear();
+			}),
 		);
 		this.onConfigurationChanged(configuration.initializingChangeEvent);
 

src/git/models/commit.ts

@@ -225,7 +225,7 @@ export abstract class GitCommit implements GitRevisionReference {
 		return GitUri.getFormattedPath(this.fileName, options);
 	}
 
-	getAvatarUri(options?: { fallback?: GravatarDefaultStyle; size?: number }): Uri | Promise<Uri> {
+	getAvatarUri(options?: { defaultStyle?: GravatarDefaultStyle; size?: number }): Uri | Promise<Uri> {
 		return getAvatarUri(this.email, this, options);
 	}
 

src/git/models/contributor.ts

@@ -20,7 +20,7 @@ export class GitContributor {
 		public readonly current: boolean = false,
 	) {}
 
-	getAvatarUri(options?: { fallback?: GravatarDefaultStyle; size?: number }): Uri | Promise<Uri> {
+	getAvatarUri(options?: { defaultStyle?: GravatarDefaultStyle; size?: number }): Uri | Promise<Uri> {
 		return getAvatarUri(this.email, undefined /*this.repoPath*/, options);
 	}
 

src/views/contributorsView.ts

@@ -1,5 +1,6 @@
 'use strict';
 import { commands, ConfigurationChangeEvent, Disposable, TreeItem, TreeItemCollapsibleState } from 'vscode';
+import { Avatars } from '../avatars';
 import { configuration, ContributorsViewConfig, ViewFilesLayout } from '../configuration';
 import { Container } from '../container';
 import { Repository, RepositoryChange, RepositoryChangeEvent } from '../git/git';
@@ -13,9 +14,8 @@ import {
 	unknownGitUri,
 	ViewNode,
 } from './nodes';
-import { debug, Functions, gate } from '../system';
+import { debug, gate } from '../system';
 import { ViewBase } from './viewBase';
-import { onDidFetchAvatar } from '../avatars';
 
 export class ContributorsRepositoryNode extends SubscribeableViewNode<ContributorsView> {
 	protected splatted = true;
@@ -75,7 +75,7 @@ export class ContributorsRepositoryNode extends SubscribeableViewNode
 	protected subscribe() {
 		return Disposable.from(
 			this.repo.onDidChange(this.onRepositoryChanged, this),
-			onDidFetchAvatar(Functions.debounce(() => this.refresh(), 500)),
+			Avatars.onDidFetch(e => this.child?.updateAvatar(e.email)),
 		);
 	}
 

src/views/nodes/commitFileNode.ts

@@ -90,7 +90,7 @@ export class CommitFileNode extends ViewRefFileNode {
 			if (!this.commit.isUncommitted && !(this.view instanceof StashesView) && this.view.config.avatars) {
 				item.iconPath = this._options.unpublished
 					? new ThemeIcon('arrow-up')
-					: await this.commit.getAvatarUri({ fallback: Container.config.defaultGravatarsStyle });
+					: await this.commit.getAvatarUri({ defaultStyle: Container.config.defaultGravatarsStyle });
 			}
 		}
 

src/views/nodes/commitNode.ts

@@ -121,7 +121,7 @@ export class CommitNode extends ViewRefNode
 		item.iconPath = this.unpublished
 			? new ThemeIcon('arrow-up')
 			: !(this.view instanceof StashesView) && this.view.config.avatars
-			? await this.commit.getAvatarUri({ fallback: Container.config.defaultGravatarsStyle })
+			? await this.commit.getAvatarUri({ defaultStyle: Container.config.defaultGravatarsStyle })
 			: new ThemeIcon('git-commit');
 		item.tooltip = this.tooltip;
 

src/views/nodes/contributorNode.ts

@@ -80,7 +80,9 @@ export class ContributorNode extends ViewNode
 		}\n${Strings.pluralize('commit', this.contributor.count)}`;
 
 		if (this.view.config.avatars) {
-			item.iconPath = await this.contributor.getAvatarUri({ fallback: Container.config.defaultGravatarsStyle });
+			item.iconPath = await this.contributor.getAvatarUri({
+				defaultStyle: Container.config.defaultGravatarsStyle,
+			});
 		}
 
 		return item;

src/views/nodes/contributorsNode.ts

@@ -19,7 +19,7 @@ export class ContributorsNode extends ViewNode
 
 	protected splatted = true;
 
-	private _children: ViewNode[] | undefined;
+	private _children: ContributorNode[] | undefined;
 
 	constructor(
 		uri: GitUri,
@@ -58,6 +58,16 @@ export class ContributorsNode extends ViewNode
 		return item;
 	}
 
+	updateAvatar(email: string) {
+		if (this._children == null) return;
+
+		for (const child of this._children) {
+			if (child.contributor.email === email) {
+				void child.triggerChange();
+			}
+		}
+	}
+
 	@gate()
 	@debug()
 	refresh() {

src/webviews/rebaseEditor.ts

@@ -156,7 +156,7 @@ export class RebaseEditorProvider implements CustomTextEditorProvider, Disposabl
 				authors.set(commit.author, {
 					author: commit.author,
 					avatarUrl: (
-						await commit.getAvatarUri({ fallback: Container.config.defaultGravatarsStyle })
+						await commit.getAvatarUri({ defaultStyle: Container.config.defaultGravatarsStyle })
 					).toString(true),
 					email: commit.email,
 				});
@@ -183,7 +183,7 @@ export class RebaseEditorProvider implements CustomTextEditorProvider, Disposabl
 				authors.set(commit.author, {
 					author: commit.author,
 					avatarUrl: (
-						await commit.getAvatarUri({ fallback: Container.config.defaultGravatarsStyle })
+						await commit.getAvatarUri({ defaultStyle: Container.config.defaultGravatarsStyle })
 					).toString(true),
 					email: commit.email,
 				});