Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(http): Fix coverage #23004

Merged
merged 8 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion lib/util/http/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,20 @@ import {
} from '../../constants';
import type { GotOptions } from './types';

export function applyAuthorization(inOptions: GotOptions): GotOptions {
type AuthGotOptions = Pick<
viceice marked this conversation as resolved.
Show resolved Hide resolved
zharinov marked this conversation as resolved.
Show resolved Hide resolved
GotOptions,
| 'hostType'
| 'headers'
| 'noAuth'
| 'context'
| 'token'
| 'username'
| 'password'
>;

export function applyAuthorization<GotOptions extends AuthGotOptions>(
inOptions: GotOptions
): GotOptions {
const options: GotOptions = { ...inOptions };

if (options.headers?.authorization || options.noAuth) {
Expand Down
8 changes: 6 additions & 2 deletions lib/util/http/bitbucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,12 @@ export class BitbucketHttp extends Http<BitbucketHttpOptions> {

// Override other page-related attributes
resultBody.pagelen = resultBody.values.length;
resultBody.size = page > MAX_PAGES ? undefined : resultBody.values.length;
resultBody.next = page > MAX_PAGES ? undefined : nextURL;
resultBody.size =
page <= MAX_PAGES
? resultBody.values.length
: /* istanbul ignore next */ undefined;
resultBody.next =
page <= MAX_PAGES ? nextURL : /* istanbul ignore next */ undefined;
}

return result;
Expand Down
4 changes: 3 additions & 1 deletion lib/util/http/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import type { Hooks, Response } from 'got';

function isResponseOk(response: Response): boolean {
const { statusCode } = response;
const limitStatusCode = response.request.options.followRedirect ? 299 : 399;
const limitStatusCode = response.request.options.followRedirect
? 299
: /* istanbul ignore next */ 399;
rarkins marked this conversation as resolved.
Show resolved Hide resolved

return (
(statusCode >= 200 && statusCode <= limitStatusCode) || statusCode === 304
Expand Down
3 changes: 2 additions & 1 deletion lib/util/http/host-rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { bootstrap } from '../../proxy';
import * as hostRules from '../host-rules';
import { dnsLookup } from './dns';
import { applyHostRules } from './host-rules';
import type { GotOptions } from './types';

const url = 'https://github.com';

jest.mock('global-agent');

describe('util/http/host-rules', () => {
const options = {
const options: GotOptions = {
hostType: 'github',
};

Expand Down
29 changes: 27 additions & 2 deletions lib/util/http/host-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,29 @@ import { dnsLookup } from './dns';
import { keepaliveAgents } from './keepalive';
import type { GotOptions } from './types';

export function findMatchingRules(options: GotOptions, url: string): HostRule {
type HostRulesGotOptions = Pick<
GotOptions,
| 'hostType'
| 'url'
| 'noAuth'
| 'headers'
| 'token'
| 'username'
| 'password'
| 'context'
| 'enabled'
| 'abortOnError'
| 'abortIgnoreStatusCodes'
| 'timeout'
| 'lookup'
| 'agent'
| 'http2'
>;

export function findMatchingRules<GotOptions extends HostRulesGotOptions>(
options: GotOptions,
url: string
): HostRule {
const { hostType } = options;
let res = hostRules.find({ hostType, url });

Expand Down Expand Up @@ -70,7 +92,10 @@ export function findMatchingRules(options: GotOptions, url: string): HostRule {
}

// Apply host rules to requests
export function applyHostRules(url: string, inOptions: GotOptions): GotOptions {
export function applyHostRules<GotOptions extends HostRulesGotOptions>(
url: string,
inOptions: GotOptions
): GotOptions {
const options: GotOptions = { ...inOptions };
const foundRules = findMatchingRules(options, url);
const { username, password, token, enabled, authType } = foundRules;
Expand Down
19 changes: 10 additions & 9 deletions lib/util/http/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import merge from 'deepmerge';
import got, { Options, RequestError } from 'got';
import hasha from 'hasha';
import type { SetRequired } from 'type-fest';
import { infer as Infer, ZodType } from 'zod';
import { HOST_DISABLED } from '../../constants/error-messages';
import { pkg } from '../../expose.cjs';
Expand Down Expand Up @@ -77,7 +78,7 @@ function applyDefaultHeaders(options: Options): void {
// `request`.
async function gotTask<T>(
url: string,
options: GotOptions,
options: SetRequired<GotOptions, 'method'>,
requestStats: Omit<RequestStats, 'duration' | 'statusCode'>
): Promise<HttpResponse<T>> {
logger.trace({ url, options }, 'got request');
Expand All @@ -102,9 +103,10 @@ async function gotTask<T>(
duration =
error.timings?.phases.total ??
/* istanbul ignore next: can't be tested */ -1;
const method = options.method?.toUpperCase() ?? 'GET';
const code = error.code ?? 'UNKNOWN';
const retryCount = error.request?.retryCount ?? -1;
const method = options.method?.toUpperCase();
zharinov marked this conversation as resolved.
Show resolved Hide resolved
const code = error.code ?? /* istanbul ignore next */ 'UNKNOWN';
const retryCount =
error.request?.retryCount ?? /* istanbul ignore next */ -1;
viceice marked this conversation as resolved.
Show resolved Hide resolved
logger.debug(
`${method} ${url} = (code=${code}, statusCode=${statusCode} retryCount=${retryCount}, duration=${duration})`
);
Expand All @@ -131,14 +133,14 @@ export class Http<Opts extends HttpOptions = HttpOptions> {

protected async request<T>(
requestUrl: string | URL,
httpOptions: InternalHttpOptions & HttpRequestOptions<T> = {}
httpOptions: InternalHttpOptions & HttpRequestOptions<T>
): Promise<HttpResponse<T>> {
let url = requestUrl.toString();
if (httpOptions?.baseUrl) {
url = resolveBaseUrl(httpOptions.baseUrl, url);
}

let options: GotOptions = merge<GotOptions>(
let options = merge<SetRequired<GotOptions, 'method'>, GotOptions>(
{
method: 'get',
...this.options,
Expand All @@ -148,8 +150,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
);

const etagCache =
httpOptions.etagCache &&
(options.method === 'get' || options.method === 'head')
httpOptions.etagCache && options.method === 'get'
rarkins marked this conversation as resolved.
Show resolved Hide resolved
? httpOptions.etagCache
: null;
if (etagCache) {
Expand Down Expand Up @@ -202,7 +203,7 @@ export class Http<Opts extends HttpOptions = HttpOptions> {
const httpTask: Task<T> = () => {
const queueDuration = Date.now() - startTime;
return gotTask(url, options, {
method: options.method ?? 'get',
method: options.method,
url,
queueDuration,
});
Expand Down