Refreshes integration session before use (#4241)

Ensures that the integration session is refreshed if it has expired
before making requests to the provider.

Also improves error handling on provider API calls by throwing specific error types based on
the response status code.

Author: axosoft-ramint

src/plus/integrations/authentication/cloudIntegrationService.ts

@@ -85,6 +85,21 @@ export class CloudIntegrationService {
 					},
 				);
 			}
+
+			if (refresh) {
+				// try once to just get the lastest token if the refresh fails, and give up if that fails too
+				const newTokenRsp = await this.connection.fetchGkApi(
+					`v1/provider-tokens/${cloudIntegrationType}`,
+					{ method: 'GET' },
+					{ organizationId: false },
+				);
+				if (newTokenRsp.ok) {
+					return (await newTokenRsp.json())?.data as Promise<
+						CloudIntegrationAuthenticationSession | undefined
+					>;
+				}
+			}
+
 			return undefined;
 		}
 

src/plus/integrations/integration.ts

@@ -327,6 +327,18 @@ export abstract class IntegrationBase<
 		return defaultValue;
 	}
 
+	@gate()
+	protected async refreshSessionIfExpired(scope?: LogScope): Promise<void> {
+		if (this._session?.expiresAt != null && this._session.expiresAt < new Date()) {
+			// The current session is expired, so get the latest from the cloud and refresh if needed
+			try {
+				await this.syncCloudConnection('connected', true);
+			} catch (ex) {
+				Logger.error(ex, scope);
+			}
+		}
+	}
+
 	@debug()
 	trackRequestException(): void {
 		this.requestExceptionCount++;
@@ -433,6 +445,8 @@ export abstract class IntegrationBase<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const issues = await this.searchProviderMyIssues(
 				this._session!,
@@ -463,6 +477,8 @@ export abstract class IntegrationBase<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const issueOrPR = this.container.cache.getIssueOrPullRequest(
 			id,
 			options?.type,
@@ -507,6 +523,8 @@ export abstract class IntegrationBase<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const issue = this.container.cache.getIssue(
 			id,
 			resource,
@@ -542,6 +560,8 @@ export abstract class IntegrationBase<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const { expiryOverride, ...opts } = options ?? {};
 
 		const currentAccount = await this.container.cache.getCurrentAccount(
@@ -574,6 +594,8 @@ export abstract class IntegrationBase<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const pr = await this.container.cache.getPullRequest(id, resource, this, () => ({
 			value: (async () => {
 				try {
@@ -604,9 +626,12 @@ export abstract class IssueIntegration<
 	@gate()
 	@debug()
 	async getAccountForResource(resource: T): Promise<Account | undefined> {
+		const scope = getLogScope();
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const account = await this.getProviderAccountForResource(this._session!, resource);
 			this.resetRequestExceptionCount();
@@ -624,9 +649,12 @@ export abstract class IssueIntegration<
 	@gate()
 	@debug()
 	async getResourcesForUser(): Promise<T[] | undefined> {
+		const scope = getLogScope();
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const resources = await this.getProviderResourcesForUser(this._session!);
 			this.resetRequestExceptionCount();
@@ -640,9 +668,12 @@ export abstract class IssueIntegration<
 
 	@debug()
 	async getProjectsForResources(resources: T[]): Promise<T[] | undefined> {
+		const scope = getLogScope();
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const projects = await this.getProviderProjectsForResources(this._session!, resources);
 			this.resetRequestExceptionCount();
@@ -669,9 +700,12 @@ export abstract class IssueIntegration<
 		project: T,
 		options?: { user?: string; filters?: IssueFilter[] },
 	): Promise<IssueShape[] | undefined> {
+		const scope = getLogScope();
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const issues = await this.getProviderIssuesForProject(this._session!, project, options);
 			this.resetRequestExceptionCount();
@@ -708,6 +742,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const author = await this.getProviderAccountForEmail(this._session!, repo, email, options);
 			this.resetRequestExceptionCount();
@@ -740,6 +776,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const author = await this.getProviderAccountForCommit(this._session!, repo, rev, options);
 			this.resetRequestExceptionCount();
@@ -768,6 +806,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const defaultBranch = this.container.cache.getRepositoryDefaultBranch(
 			repo,
 			this,
@@ -805,6 +845,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const metadata = this.container.cache.getRepositoryMetadata(
 			repo,
 			this,
@@ -845,6 +887,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return false;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const result = await this.mergeProviderPullRequest(this._session!, pr, options);
 			this.resetRequestExceptionCount();
@@ -877,6 +921,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const { expiryOverride, ...opts } = options ?? {};
 
 		const pr = this.container.cache.getPullRequestForBranch(
@@ -920,6 +966,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const pr = this.container.cache.getPullRequestForSha(
 			rev,
 			repo,
@@ -954,10 +1002,13 @@ export abstract class HostingIntegration<
 			customUrl?: string;
 		},
 	): Promise<PagedResult<ProviderIssue> | undefined> {
+		const scope = getLogScope();
 		const providerId = this.authProvider.id;
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const api = await this.getProvidersApi();
 		if (
 			providerId !== HostingIntegrationId.GitLab &&
@@ -1157,10 +1208,13 @@ export abstract class HostingIntegration<
 			customUrl?: string;
 		},
 	): Promise<PagedResult<ProviderPullRequest> | undefined> {
+		const scope = getLogScope();
 		const providerId = this.authProvider.id;
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const api = await this.getProvidersApi();
 		if (
 			providerId !== HostingIntegrationId.GitLab &&
@@ -1319,6 +1373,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		const start = Date.now();
 		try {
 			const pullRequests = await this.searchProviderMyPullRequests(
@@ -1361,6 +1417,8 @@ export abstract class HostingIntegration<
 		const connected = this.maybeConnected ?? (await this.isConnected());
 		if (!connected) return undefined;
 
+		await this.refreshSessionIfExpired(scope);
+
 		try {
 			const prs = await this.searchProviderPullRequests?.(
 				this._session!,

src/plus/integrations/providers/providersApi.ts

@@ -10,6 +10,7 @@ import {
 	AuthenticationError,
 	AuthenticationErrorReason,
 	RequestClientError,
+	RequestNotFoundError,
 	RequestRateLimitError,
 } from '../../../errors';
 import type { PagedResult } from '../../../git/gitProvider';
@@ -378,15 +379,17 @@ export class ProvidersApi {
 			throw new Error(`Provider with id ${providerId} not registered`);
 		}
 
-		switch (providerId) {
-			case IssueIntegrationId.Jira: {
-				if (error?.response?.status != null) {
-					if (error.response.status === 401) {
-						throw new AuthenticationError(providerId, AuthenticationErrorReason.Forbidden, error);
-					} else if (error.response.status === 429) {
+		if (error?.response?.status != null) {
+			switch (error.response.status) {
+				case 404: // Not found
+				case 410: // Gone
+				case 422: // Unprocessable Entity
+					throw new RequestNotFoundError(error);
+				case 401: // Unauthorized
+					if (error.message?.includes('rate limit')) {
 						let resetAt: number | undefined;
 
-						const reset = error.response.headers?.['x-ratelimit-reset'];
+						const reset = error.response?.headers?.['x-ratelimit-reset'];
 						if (reset != null) {
 							resetAt = parseInt(reset, 10);
 							if (Number.isNaN(resetAt)) {
@@ -395,16 +398,32 @@ export class ProvidersApi {
 						}
 
 						throw new RequestRateLimitError(error, token, resetAt);
-					} else if (error.response.status >= 400 && error.response.status < 500) {
-						throw new RequestClientError(error);
 					}
+					throw new AuthenticationError(providerId, AuthenticationErrorReason.Unauthorized, error);
+				case 403: // Forbidden
+					throw new AuthenticationError(providerId, AuthenticationErrorReason.Forbidden, error);
+				case 429: {
+					// Too Many Requests
+					let resetAt: number | undefined;
+
+					const reset = error.response.headers?.['x-ratelimit-reset'];
+					if (reset != null) {
+						resetAt = parseInt(reset, 10);
+						if (Number.isNaN(resetAt)) {
+							resetAt = undefined;
+						}
+					}
+
+					throw new RequestRateLimitError(error, token, resetAt);
 				}
-				throw error;
-			}
-			default: {
-				throw error;
+				default:
+					if (error.response.status >= 400 && error.response.status < 500) {
+						throw new RequestClientError(error);
+					}
 			}
 		}
+
+		throw error;
 	}
 
 	async getPagedResult<T>(