Skip to content

Commit

Permalink
Handle repositories that return 404 when all tags are removed (#3043)
Browse files Browse the repository at this point in the history
Fixes #2968
  • Loading branch information
karolz-ms authored Jul 7, 2021
1 parent 50c8702 commit 6f6d020
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 30 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions src/tree/registries/dockerV2/DockerV2RepositoryTreeItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { AzExtTreeItem, IActionContext } from "vscode-azureextensionui";
import { PAGE_SIZE } from "../../../constants";
import { IOAuthContext, RequestLike } from "../../../utils/httpRequest";
import { ErrorHandling, HttpErrorResponse, HttpStatusCode, IOAuthContext, RequestLike } from "../../../utils/httpRequest";
import { getNextLinkFromHeaders, registryRequest } from "../../../utils/registryRequestUtils";
import { IAuthProvider } from "../auth/IAuthProvider";
import { ICachedRegistryProvider } from "../ICachedRegistryProvider";
Expand All @@ -29,7 +29,16 @@ export class DockerV2RepositoryTreeItem extends RemoteRepositoryTreeItemBase imp
}

const url = this._nextLink || `v2/${this.repoName}/tags/list?n=${PAGE_SIZE}`;
const response = await registryRequest<ITags>(this, 'GET', url);
const response = await registryRequest<ITags>(this, 'GET', url, undefined, ErrorHandling.ReturnErrorResponse);
if (response.status === HttpStatusCode.NotFound) {
// Some registries return 404 when all tags have been removed and the repository becomes effectively unavailable.
void this.deleteTreeItem(context);
return [];
}
else if (!response.ok) {
throw new HttpErrorResponse(response);
}

this._nextLink = getNextLinkFromHeaders(response);
return await this.createTreeItemsWithErrorHandling(
response.body.tags,
Expand Down
61 changes: 43 additions & 18 deletions src/utils/httpRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,17 @@ import { default as fetch, Request, RequestInit, Response } from 'node-fetch';
import { URL, URLSearchParams } from 'url';
import { localize } from '../localize';

export async function httpRequest<T>(url: string, options?: RequestOptionsLike, signRequest?: (request: RequestLike) => Promise<RequestLike>): Promise<HttpResponse<T>> {
export const enum ErrorHandling {
ThrowOnError,
ReturnErrorResponse
}

export async function httpRequest<T>(
url: string,
options?: RequestOptionsLike,
signRequest?: (request: RequestLike) => Promise<RequestLike>,
errorHandling: ErrorHandling = ErrorHandling.ThrowOnError
): Promise<HttpResponse<T>> {
const requestOptions: RequestInit = options;
if (options.form) {
// URLSearchParams is a silly way to say "it's form data"
Expand All @@ -23,18 +33,37 @@ export async function httpRequest<T>(url: string, options?: RequestOptionsLike,

const response = await fetch(request);

if (response.status >= 200 && response.status < 300) {
return new HttpResponse(response);
if (errorHandling === ErrorHandling.ReturnErrorResponse || response.ok) {
return new HttpResponse(response, url);
} else {
throw new HttpErrorResponse(response);
}
}

export class HttpResponse<T> {
export class HttpResponse<T> implements ResponseLike {
private bodyPromise: Promise<T> | undefined;
private normalizedHeaders: { [key: string]: string } | undefined;
public readonly headers: HeadersLike;
public readonly status: number;
public readonly statusText: string;
public readonly ok: boolean;

public constructor(private readonly innerResponse: Response, public readonly url: string) {
// Unfortunately Typescript will not consider a getter accessor when checking whether a class implements an interface.
// So we are forced to use readonly members to implement ResponseLike interface.
const headerStore: { [key: string]: string } = {};
for (const key of this.innerResponse.headers.keys()) {
headerStore[key] = this.innerResponse.headers.get(key);
}

public constructor(private readonly innerResponse: Response) { }
this.headers = {
get: (key: string) => headerStore[key],
set: (key: string, value: string) => { headerStore[key] = value; }
};

this.status = this.innerResponse.status;
this.statusText = this.innerResponse.statusText;
this.ok = this.innerResponse.ok;
}

public async json(): Promise<T> {
if (!this.bodyPromise) {
Expand All @@ -44,17 +73,6 @@ export class HttpResponse<T> {

return this.bodyPromise;
}

public get headers(): { [key: string]: string } {
if (!this.normalizedHeaders) {
this.normalizedHeaders = {};
for (const key of this.innerResponse.headers.keys()) {
this.normalizedHeaders[key] = this.innerResponse.headers.get(key);
}
}

return this.normalizedHeaders;
}
}

export class HttpErrorResponse extends Error {
Expand All @@ -70,6 +88,12 @@ export class HttpErrorResponse extends Error {

type RequestMethod = 'HEAD' | 'GET' | 'POST' | 'DELETE' | 'PUT' | 'PATCH';

// Contains only status codes that we handle explicitly in the extension code
export const enum HttpStatusCode {
Unauthorized = 401,
NotFound = 404
}

export interface RequestOptionsLike {
headers?: { [key: string]: string };
method?: RequestMethod; // This is an enum type because it enforces the above valid options on callers (which do not directly use node-fetch's Request object)
Expand All @@ -92,6 +116,7 @@ export interface ResponseLike {
url: string;
status: number;
statusText: string;
ok: boolean;
}

export async function streamToFile(downloadUrl: string, fileName: string): Promise<void> {
Expand Down Expand Up @@ -131,7 +156,7 @@ const serviceRegExp = /service="([^"]+)"/i;
const scopeRegExp = /scope="([^"]+)"/i;

export function getWwwAuthenticateContext(error: HttpErrorResponse): IOAuthContext | undefined {
if (error.response?.status === 401) {
if (error.response?.status === HttpStatusCode.Unauthorized) {
const wwwAuthHeader: string | undefined = error.response?.headers?.get('www-authenticate') as string;

const realmMatch = wwwAuthHeader?.match(realmRegExp);
Expand Down
23 changes: 14 additions & 9 deletions src/utils/registryRequestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

import { URL } from "url";
import { ociClientId } from "../constants";
import { httpRequest, RequestLike } from './httpRequest';
import { ErrorHandling, httpRequest, RequestLike, ResponseLike } from './httpRequest';

export function getNextLinkFromHeaders(response: IResponse<unknown>): string | undefined {
const linkHeader: string | undefined = response.headers.link as string;
export function getNextLinkFromHeaders(response: IRegistryRequestResponse<unknown>): string | undefined {
const linkHeader: string | undefined = response.headers.get('link') as string;
if (linkHeader) {
const match = linkHeader.match(/<(.*)>; rel="next"/i);
return match ? match[1] : undefined;
Expand All @@ -17,7 +17,13 @@ export function getNextLinkFromHeaders(response: IResponse<unknown>): string | u
}
}

export async function registryRequest<T>(node: IRegistryAuthTreeItem | IRepositoryAuthTreeItem, method: 'GET' | 'DELETE' | 'POST', url: string, customOptions?: RequestInit): Promise<IResponse<T>> {
export async function registryRequest<T>(
node: IRegistryAuthTreeItem | IRepositoryAuthTreeItem,
method: 'GET' | 'DELETE' | 'POST',
url: string,
customOptions?: RequestInit,
errorHandling: ErrorHandling = ErrorHandling.ThrowOnError
): Promise<IRegistryRequestResponse<T>> {
const options = {
method: method,
headers: {
Expand All @@ -39,17 +45,16 @@ export async function registryRequest<T>(node: IRegistryAuthTreeItem | IReposito
} else {
return (<IRepositoryAuthTreeItem>node).parent?.signRequest(request);
}
});
}, errorHandling);

return {
body: method !== 'DELETE' ? await response.json() : undefined,
headers: response.headers,
...response
};
}

interface IResponse<T> {
body: T,
headers: { [key: string]: string | string[] },
export interface IRegistryRequestResponse<T> extends ResponseLike {
body: T
}

export interface IRegistryAuthTreeItem {
Expand Down

0 comments on commit 6f6d020

Please sign in to comment.