Przeglądaj źródła

Adds cancellation & timeout to fetch requests

Adds default (60s) timeout to fetch requests
Refs #3013
main
Eric Amodio 1 rok temu
rodzic
commit
db94de8d9b
12 zmienionych plików z 96 dodań i 53 usunięć
  1. +5
    -0
      .eslintrc.base.json
  2. +3
    -3
      src/env/node/git/git.ts
  3. +5
    -7
      src/env/node/git/localGitProvider.ts
  4. +2
    -1
      src/errors.ts
  5. +2
    -2
      src/plus/drafts/draftsService.ts
  6. +2
    -2
      src/plus/gk/account/authenticationConnection.ts
  7. +6
    -1
      src/plus/gk/account/authenticationProvider.ts
  8. +2
    -2
      src/plus/gk/account/subscriptionService.ts
  9. +65
    -31
      src/plus/gk/serverConnection.ts
  10. +2
    -2
      src/plus/integrations/providerIntegration.ts
  11. +1
    -1
      src/plus/integrations/providers/github/github.ts
  12. +1
    -1
      src/plus/integrations/providers/gitlab/gitlab.ts

+ 5
- 0
.eslintrc.base.json Wyświetl plik

@ -96,6 +96,11 @@
"group": ["react-dom"],
"importNames": ["Container"],
"message": "Use our Container instead"
},
{
"group": ["vscode"],
"importNames": ["CancellationError"],
"message": "Use our CancellationError instead"
}
]
}

+ 3
- 3
src/env/node/git/git.ts Wyświetl plik

@ -284,9 +284,9 @@ export class Git {
}
if (cancellation) {
const controller = new AbortController();
spawnOpts.signal = controller.signal;
cancellation.onCancellationRequested(() => controller.abort());
const aborter = new AbortController();
spawnOpts.signal = aborter.signal;
cancellation.onCancellationRequested(() => aborter.abort());
}
const proc = spawn(await this.path(), args, spawnOpts);

+ 5
- 7
src/env/node/git/localGitProvider.ts Wyświetl plik

@ -648,13 +648,11 @@ export class LocalGitProvider implements GitProvider, Disposable {
// Check if the url returns a 200 status code
let promise = this._pendingRemoteVisibility.get(url);
if (promise == null) {
const cancellation = new AbortController();
let timeout: ReturnType<typeof setTimeout>;
promise = fetch(url, { method: 'HEAD', agent: getProxyAgent(), signal: cancellation.signal }).then(r => {
clearTimeout(timeout);
return r;
});
timeout = setTimeout(() => cancellation.abort(), 30000);
const aborter = new AbortController();
const timer = setTimeout(() => aborter.abort(), 30000);
promise = fetch(url, { method: 'HEAD', agent: getProxyAgent(), signal: aborter.signal });
void promise.finally(() => clearTimeout(timer));
this._pendingRemoteVisibility.set(url, promise);
}

+ 2
- 1
src/errors.ts Wyświetl plik

@ -1,4 +1,5 @@
import type { Uri } from 'vscode';
// eslint-disable-next-line no-restricted-imports
import { CancellationError as _CancellationError } from 'vscode';
import type { Response } from '@env/fetch';
import type { RequiredSubscriptionPlans, Subscription } from './plus/gk/account/subscription';
@ -95,7 +96,7 @@ export class AuthenticationRequiredError extends Error {
}
export class CancellationError extends _CancellationError {
constructor() {
constructor(public readonly original?: Error) {
super();
Error.captureStackTrace?.(this, CancellationError);

+ 2
- 2
src/plus/drafts/draftsService.ts Wyświetl plik

@ -121,7 +121,7 @@ export class DraftService implements Disposable {
const files = diffFiles?.files.map(f => ({ ...f, gkRepositoryId: patch.gitRepositoryId })) ?? [];
// Upload patch to returned S3 url
await this.connection.fetchRaw(url, {
await this.connection.fetch(url, {
method: method,
headers: {
'Content-Type': 'text/plain',
@ -528,7 +528,7 @@ export class DraftService implements Disposable {
const { url, method, headers } = secureLink;
// Download patch from returned S3 url
const contentsRsp = await this.connection.fetchRaw(url, {
const contentsRsp = await this.connection.fetch(url, {
method: method,
headers: {
Accept: 'text/plain',

+ 2
- 2
src/plus/gk/account/authenticationConnection.ts Wyświetl plik

@ -38,13 +38,13 @@ export class AuthenticationConnection implements Disposable {
return new Promise<void>(resolve => setTimeout(resolve, 50));
}
@debug({ args: false })
@debug<AuthenticationConnection['getAccountInfo']>({ args: false, exit: r => `returned ${r.id}` })
async getAccountInfo(token: string): Promise<AccountInfo> {
const scope = getLogScope();
let rsp: Response;
try {
rsp = await this.connection.fetchApi('user', undefined, token);
rsp = await this.connection.fetchApi('user', undefined, { token: token });
} catch (ex) {
Logger.error(ex, scope);
throw ex;

+ 6
- 1
src/plus/gk/account/authenticationProvider.ts Wyświetl plik

@ -6,6 +6,7 @@ import type {
import { authentication, Disposable, EventEmitter, window } from 'vscode';
import { uuid } from '@env/crypto';
import type { Container, Environment } from '../../../container';
import { CancellationError } from '../../../errors';
import { debug } from '../../../system/decorators/log';
import { Logger } from '../../../system/logger';
import { getLogScope, setLogScopeExit } from '../../../system/logger.scope';
@ -96,7 +97,11 @@ export class AccountAuthenticationProvider implements AuthenticationProvider, Di
if (ex === 'Cancelled') throw ex;
Logger.error(ex, scope);
void window.showErrorMessage(`Unable to sign in to GitKraken: ${ex}`);
void window.showErrorMessage(
`Unable to sign in to GitKraken: ${
ex instanceof CancellationError ? 'request timed out' : ex
}. Please try again. If this issue persists, please contact support.`,
);
throw ex;
}
}

+ 2
- 2
src/plus/gk/account/subscriptionService.ts Wyświetl plik

@ -363,7 +363,7 @@ export class SubscriptionService implements Disposable {
method: 'POST',
body: JSON.stringify({ id: session.account.id }),
},
session.accessToken,
{ token: session.accessToken },
);
if (!rsp.ok) {
@ -570,7 +570,7 @@ export class SubscriptionService implements Disposable {
method: 'POST',
body: JSON.stringify(checkInData),
},
session.accessToken,
{ token: session.accessToken },
);
if (!rsp.ok) {

+ 65
- 31
src/plus/gk/serverConnection.ts Wyświetl plik

@ -1,13 +1,22 @@
import type { Disposable } from 'vscode';
import type { CancellationToken, Disposable } from 'vscode';
import { Uri } from 'vscode';
import type { RequestInfo, RequestInit, Response } from '@env/fetch';
import { fetch as _fetch, getProxyAgent } from '@env/fetch';
import type { Container } from '../../container';
import { AuthenticationRequiredError } from '../../errors';
import { AuthenticationRequiredError, CancellationError } from '../../errors';
import { memoize } from '../../system/decorators/memoize';
import { Logger } from '../../system/logger';
import { getLogScope } from '../../system/logger.scope';
interface FetchOptions {
cancellation?: CancellationToken;
timeout?: number;
}
interface GKFetchOptions extends FetchOptions {
token?: string;
}
export class ServerConnection implements Disposable {
constructor(private readonly container: Container) {}
@ -96,60 +105,85 @@ export class ServerConnection implements Disposable {
return 'Visual-Studio-Code-GitLens';
}
async fetch(url: RequestInfo, init?: RequestInit, token?: string): Promise<Response> {
async fetch(url: RequestInfo, init?: RequestInit, options?: FetchOptions): Promise<Response> {
const scope = getLogScope();
if (options?.cancellation?.isCancellationRequested) throw new CancellationError();
const aborter = new AbortController();
let timeout;
if (options?.cancellation != null) {
timeout = options.timeout; // Don't set a default timeout if we have a cancellation token
options.cancellation.onCancellationRequested(() => aborter.abort());
} else {
timeout = options?.timeout ?? 60 * 1000;
}
const timer = timeout != null ? setTimeout(() => aborter.abort(), timeout) : undefined;
try {
token ??= await this.getAccessToken();
const options = {
const promise = _fetch(url, {
agent: getProxyAgent(),
...init,
headers: {
Authorization: `Bearer ${token}`,
'User-Agent': this.userAgent,
'Content-Type': 'application/json',
...init?.headers,
},
};
// TODO@eamodio handle common response errors
return await _fetch(url, options);
signal: aborter?.signal,
});
void promise.finally(() => clearTimeout(timer));
return await promise;
} catch (ex) {
Logger.error(ex, scope);
if (ex.name === 'AbortError') throw new CancellationError(ex);
throw ex;
}
}
async fetchApi(path: string, init?: RequestInit, token?: string): Promise<Response> {
return this.fetch(this.getApiUrl(path), init, token);
async fetchApi(path: string, init?: RequestInit, options?: GKFetchOptions): Promise<Response> {
return this.gkFetch(this.getApiUrl(path), init, options);
}
async fetchApiGraphQL(path: string, request: GraphQLRequest, init?: RequestInit) {
return this.fetchApi(path, {
method: 'POST',
...init,
body: JSON.stringify(request),
});
async fetchApiGraphQL(path: string, request: GraphQLRequest, init?: RequestInit, options?: GKFetchOptions) {
return this.fetchApi(
path,
{
method: 'POST',
...init,
body: JSON.stringify(request),
},
options,
);
}
async fetchGkDevApi(path: string, init?: RequestInit, token?: string): Promise<Response> {
return this.fetch(this.getGkDevApiUrl(path), init, token);
async fetchGkDevApi(path: string, init?: RequestInit, options?: GKFetchOptions): Promise<Response> {
return this.gkFetch(this.getGkDevApiUrl(path), init, options);
}
async fetchRaw(url: RequestInfo, init?: RequestInit): Promise<Response> {
private async gkFetch(url: RequestInfo, init?: RequestInit, options?: GKFetchOptions): Promise<Response> {
const scope = getLogScope();
try {
const options = {
agent: getProxyAgent(),
...init,
headers: {
'User-Agent': this.userAgent,
...init?.headers,
let token;
({ token, ...options } = options ?? {});
token ??= await this.getAccessToken();
// TODO@eamodio handle common response errors
return this.fetch(
url,
{
...init,
headers: {
Authorization: `Bearer ${token}`,
'Content-Type': 'application/json',
...init?.headers,
},
},
};
return await _fetch(url, options);
options,
);
} catch (ex) {
Logger.error(ex, scope);
throw ex;

+ 2
- 2
src/plus/integrations/providerIntegration.ts Wyświetl plik

@ -1,7 +1,7 @@
import type { AuthenticationSession, CancellationToken, Event, MessageItem } from 'vscode';
import { CancellationError, EventEmitter, window } from 'vscode';
import { EventEmitter, window } from 'vscode';
import type { Container } from '../../container';
import { AuthenticationError, ProviderRequestClientError } from '../../errors';
import { AuthenticationError, CancellationError, ProviderRequestClientError } from '../../errors';
import type { PagedResult } from '../../git/gitProvider';
import type { Account } from '../../git/models/author';
import type { DefaultBranch } from '../../git/models/defaultBranch';

+ 1
- 1
src/plus/integrations/providers/github/github.ts Wyświetl plik

@ -2463,7 +2463,7 @@ export class GitHubApi implements Disposable {
ex: RequestError | (Error & { name: 'AbortError' }),
scope: LogScope | undefined,
): void {
if (ex.name === 'AbortError') throw new CancellationError();
if (ex.name === 'AbortError') throw new CancellationError(ex);
switch (ex.status) {
case 404: // Not found

+ 1
- 1
src/plus/integrations/providers/gitlab/gitlab.ts Wyświetl plik

@ -879,7 +879,7 @@ $search: String!
ex: ProviderFetchError | (Error & { name: 'AbortError' }),
scope: LogScope | undefined,
): void {
if (ex.name === 'AbortError' || !(ex instanceof ProviderFetchError)) throw new CancellationError();
if (ex.name === 'AbortError' || !(ex instanceof ProviderFetchError)) throw new CancellationError(ex);
switch (ex.status) {
case 404: // Not found

Ładowanie…
Anuluj
Zapisz