Skip to content

Commit

Permalink
feat(gitlab): retry requests on resource locks (#29019)
Browse files Browse the repository at this point in the history
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
  • Loading branch information
fgreinacher and viceice committed May 14, 2024
1 parent 23421c5 commit c608cee
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 1 deletion.
41 changes: 41 additions & 0 deletions lib/util/http/gitlab.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { HTTPError } from 'got';
import * as httpMock from '../../../test/http-mock';
import { EXTERNAL_HOST_ERROR } from '../../constants/error-messages';
import { GitlabReleasesDatasource } from '../../modules/datasource/gitlab-releases';
Expand Down Expand Up @@ -144,4 +145,44 @@ describe('util/http/gitlab', () => {
);
});
});

describe('handles 409 errors', () => {
let NODE_ENV: string | undefined;

beforeAll(() => {
// Unset NODE_ENV so that we can test the retry logic
NODE_ENV = process.env.NODE_ENV;
delete process.env.NODE_ENV;
});

afterAll(() => {
process.env.NODE_ENV = NODE_ENV;
});

it('retries the request on resource lock', async () => {
const body = { message: '409 Conflict: Resource lock' };
httpMock.scope(gitlabApiHost).post('/api/v4/some-url').reply(409, body);
httpMock.scope(gitlabApiHost).post('/api/v4/some-url').reply(200, {});
const res = await gitlabApi.postJson('some-url', {});
expect(res.statusCode).toBe(200);
});

it('does not retry more than twice on resource lock', async () => {
const body = { message: '409 Conflict: Resource lock' };
httpMock.scope(gitlabApiHost).post('/api/v4/some-url').reply(409, body);
httpMock.scope(gitlabApiHost).post('/api/v4/some-url').reply(409, body);
httpMock.scope(gitlabApiHost).post('/api/v4/some-url').reply(409, body);
await expect(gitlabApi.postJson('some-url', {})).rejects.toThrow(
HTTPError,
);
});

it('does not retry for other reasons', async () => {
const body = { message: 'Other reason' };
httpMock.scope(gitlabApiHost).post('/api/v4/some-url').reply(409, body);
await expect(gitlabApi.postJson('some-url', {})).rejects.toThrow(
HTTPError,
);
});
});
});
16 changes: 16 additions & 0 deletions lib/util/http/gitlab.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import is from '@sindresorhus/is';
import type { RetryObject } from 'got';
import { logger } from '../../logger';
import { ExternalHostError } from '../../types/errors/external-host-error';
import { parseLinkHeader, parseUrl } from '../url';
Expand Down Expand Up @@ -83,4 +84,19 @@ export class GitlabHttp extends Http<GitlabHttpOptions> {
throw err;
}
}

protected override calculateRetryDelay(retryObject: RetryObject): number {
const { error, attemptCount, retryOptions } = retryObject;
if (
attemptCount <= retryOptions.limit &&
error.options.method === 'POST' &&
error.response?.statusCode === 409 &&
error.response.rawBody.toString().includes('Resource lock')
) {
const noise = Math.random() * 100;
return 2 ** (attemptCount - 1) * 1000 + noise;
}

return super.calculateRetryDelay(retryObject);
}
}
8 changes: 7 additions & 1 deletion lib/util/http/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import is from '@sindresorhus/is';
import merge from 'deepmerge';
import got, { Options, RequestError } from 'got';
import got, { Options, RequestError, RetryObject } from 'got';
import type { SetRequired } from 'type-fest';
import { infer as Infer, type ZodError, ZodType } from 'zod';
import { GlobalConfig } from '../../config/global';
Expand Down Expand Up @@ -124,6 +124,8 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
{
context: { hostType },
retry: {
calculateDelay: (retryObject) =>
this.calculateRetryDelay(retryObject),
limit: retryLimit,
maxRetryAfter: 0, // Don't rely on `got` retry-after handling, just let it fail and then we'll handle it
},
Expand Down Expand Up @@ -248,6 +250,10 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
}
}

protected calculateRetryDelay({ computedValue }: RetryObject): number {
return computedValue;
}

get(url: string, options: HttpOptions = {}): Promise<HttpResponse> {
return this.request<string>(url, options);
}
Expand Down

0 comments on commit c608cee

Please sign in to comment.