Concurrent 401s may trigger one token refresh per in-flight request instead of one per expired token

[dpusceddu]

We are happy to answer your questions about the code or discuss technical ideas.

Please complete the following checklist (by adding [x]):

• I have searched open and closed issues for duplicates
• This isn't a feature request
• This is not a report about my app not working as expected

---

While searching more in-depth for whatever caused the failed token refreshes mentioned in #43 I found the following. The 401 retry hook refreshes the session once per straggling in-flight request instead of once per expired access token. Although there is a guard that dedupes refreshes that overlap in time:

sdk/incubating/account/js/src/apiClient.ts

Lines 110 to 115 in f249616

	async refreshSessionIfPossible(): Promise<boolean> {
	this.activeRefreshPromise ??= this.performTokenRefresh().finally(() => {
	this.activeRefreshPromise = null;
	});
	return this.activeRefreshPromise;
	}

The hook never checks whether the 401'd request was sent with the current access token:

sdk/incubating/account/js/src/apiClient.ts

Lines 82 to 108 in f249616

	private createRefreshSessionAfterResponseHook(): AfterResponseHook {
	return async (request, options, response) => {
	if (response.status !== 401 || shouldSkipAuthRefreshForUrl(request.url)) {
	return;
	}
	this.options.logger.info('Refreshing session');
	const refreshed = await this.refreshSessionIfPossible();
	if (!refreshed) {
	return;
	}
	const uid = this.options.credentials.uid;
	const accessToken = this.options.credentials.accessToken;
	if (!uid || !accessToken) {
	return;
	}
	const headers = new Headers(options.headers);
	headers.set('x-pm-appversion', this.options.appVersion);
	headers.set('x-pm-uid', uid);
	headers.set('Authorization', `Bearer ${accessToken}`);
	return this.authenticatedClient(request, { ...options, headers });
	};
	}

Hence, with multiple parallel requests in flight when the access token expires, the first 401 correctly triggers a refresh and rotates the token, and stragglers whose 401s arrive during that refresh are correctly deduped by the guard above. But any straggler whose 401 lands after the refresh completed triggers a new refresh with the just-rotated refresh token, rotating again.

That's my best guess for what happened in the following journalctl excerpt. Each keyring rewrite showed up in the journal as one gnome-keyring replace warning during a nightly upload that ran fine until access-token expiry.

Jun 29 04:17:00 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:01 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:07 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:12 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:13 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:18 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:19 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:19 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:17:24 fedora gnome-keyring-daemon[4506]: asked to register item /org/freedesktop/secrets/collection/login/1, but it's already registered
Jun 29 04:18:55 fedora systemd[2430]: immich-proton-backup.service: Main process exited, code=exited, status=1/FAILURE

I believe a good approach is for the hook compare the token of the failed request (in its Authorization header) with the currently active token credentials.accessToken and skip the refresh if it is different (has already been refreshed).