From e02f0be796ef432e6c06daa155dc215a6ce6398d Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 4 Jan 2022 12:09:47 +0000 Subject: [PATCH] fix: improve the fetch handling (and testing) of requests to the Cloudflare API --- .changeset/modern-books-leave.md | 17 ++ .changeset/thick-beans-thank.md | 9 + packages/wrangler/src/__tests__/jest.setup.ts | 6 +- packages/wrangler/src/__tests__/kv.test.ts | 73 +++++--- .../wrangler/src/__tests__/mock-cfetch.ts | 162 +++++++++++++++--- packages/wrangler/src/api/preview.ts | 8 +- packages/wrangler/src/cfetch.ts | 72 -------- packages/wrangler/src/cfetch/index.ts | 103 +++++++++++ packages/wrangler/src/cfetch/internal.ts | 69 ++++++++ packages/wrangler/src/index.tsx | 26 ++- packages/wrangler/src/kv.tsx | 57 +++--- packages/wrangler/src/publish.ts | 30 ++-- packages/wrangler/src/sites.tsx | 11 +- packages/wrangler/src/tail.tsx | 8 +- 14 files changed, 458 insertions(+), 193 deletions(-) create mode 100644 .changeset/modern-books-leave.md create mode 100644 .changeset/thick-beans-thank.md delete mode 100644 packages/wrangler/src/cfetch.ts create mode 100644 packages/wrangler/src/cfetch/index.ts create mode 100644 packages/wrangler/src/cfetch/internal.ts diff --git a/.changeset/modern-books-leave.md b/.changeset/modern-books-leave.md new file mode 100644 index 0000000000000..af7a99c9e31a8 --- /dev/null +++ b/.changeset/modern-books-leave.md @@ -0,0 +1,17 @@ +--- +"wrangler": patch +--- + +Refactor raw value extraction from Cloudflare APIs + +Most API responses are JSON of the form: + +``` +{ result, success, errors, messages, result_info } +``` + +where the `result` contains the actual response value. + +But some API responses only contain the result value. + +This change refactors the client-side fetch API to allow callers to specify what kind of response they expect. diff --git a/.changeset/thick-beans-thank.md b/.changeset/thick-beans-thank.md new file mode 100644 index 0000000000000..f20def44ed0ff --- /dev/null +++ b/.changeset/thick-beans-thank.md @@ -0,0 +1,9 @@ +--- +"wrangler": patch +--- + +Fix pagination handling of list requests to the Cloudflare API + +When doing a list request to the API, the server may respond with only a single page of results. +In this case, it will also provide a `cursor` value in the `result_info` part of the response, which can be used to request the next page. +This change implements this on the client-side so that we get all the results by requesting further pages when there is a cursor. diff --git a/packages/wrangler/src/__tests__/jest.setup.ts b/packages/wrangler/src/__tests__/jest.setup.ts index 1c51c5a52f3bf..6575a481e2ff5 100644 --- a/packages/wrangler/src/__tests__/jest.setup.ts +++ b/packages/wrangler/src/__tests__/jest.setup.ts @@ -1,6 +1,10 @@ +import { mockFetchInternal } from "./mock-cfetch"; import { confirm, prompt } from "../dialogs"; +import { fetchInternal } from "../cfetch/internal"; + +jest.mock("../cfetch/internal"); +(fetchInternal as jest.Mock).mockImplementation(mockFetchInternal); -jest.mock("../cfetch", () => jest.requireActual("./mock-cfetch")); jest.mock("../dialogs"); // By default (if not configured by mockConfirm()) calls to `confirm()` should throw. diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts index 4248f41ed4373..52815cbc1ab95 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/packages/wrangler/src/__tests__/kv.test.ts @@ -1,7 +1,11 @@ -import { setMock, unsetAllMocks } from "./mock-cfetch"; +import { writeFileSync } from "fs"; +import { + setMockResponse, + setMockRawResponse, + unsetAllMocks, +} from "./mock-cfetch"; import { runWrangler } from "./run-wrangler"; import { runInTempDir } from "./run-in-tmp"; -import { writeFileSync } from "fs"; describe("wrangler", () => { runInTempDir(); @@ -13,14 +17,19 @@ describe("wrangler", () => { describe("kv:namespace", () => { describe("create", () => { function mockCreateRequest(expectedTitle: string) { - setMock( + setMockResponse( "/accounts/:accountId/storage/kv/namespaces", "POST", ([_url, accountId], { body }) => { expect(accountId).toEqual("some-account-id"); - const title = JSON.parse(body).title; + const title = JSON.parse(body as string).title; expect(title).toEqual(expectedTitle); - return { id: "some-namespace-id" }; + return { + success: true, + result: { id: "some-namespace-id" }, + errors: [], + messages: [], + }; } ); } @@ -172,19 +181,24 @@ describe("wrangler", () => { describe("list", () => { function mockListRequest(namespaces: unknown[]) { const requests = { count: 0 }; - setMock( - "/accounts/:accountId/storage/kv/namespaces\\?:qs", - ([_url, accountId, query], init) => { + setMockResponse( + "/accounts/:accountId/storage/kv/namespaces", + ([_url, accountId], init, query) => { requests.count++; expect(accountId).toEqual("some-account-id"); - expect(query).toContain("per_page=100"); - expect(query).toContain("order=title"); - expect(query).toContain("direction=asc"); - expect(query).toContain("page="); - expect(init).toBe(undefined); - const pageSize = Number(/\bper_page=(\d+)\b/.exec(query)[1]); - const page = Number(/\bpage=(\d+)/.exec(query)[1]); - return namespaces.slice((page - 1) * pageSize, page * pageSize); + expect(query.get("per_page")).toEqual("100"); + expect(query.get("order")).toEqual("title"); + expect(query.get("direction")).toEqual("asc"); + expect(query.get("page")).toEqual(`${requests.count}`); + expect(init).toEqual({}); + const pageSize = Number(query.get("per_page")); + const page = Number(query.get("page")); + return { + success: true, + result: namespaces.slice((page - 1) * pageSize, page * pageSize), + errors: [], + messages: [], + }; } ); return requests; @@ -196,7 +210,11 @@ describe("wrangler", () => { { title: "title-2", id: "id-2" }, ]; mockListRequest(KVNamespaces); - const { stdout } = await runWrangler("kv:namespace list"); + const { error, stdout, stderr } = await runWrangler( + "kv:namespace list" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); const namespaces = JSON.parse(stdout); expect(namespaces).toEqual(KVNamespaces); }); @@ -218,7 +236,7 @@ describe("wrangler", () => { describe("delete", () => { function mockDeleteRequest(expectedNamespaceId: string) { const requests = { count: 0 }; - setMock( + setMockResponse( "/accounts/:accountId/storage/kv/namespaces/:namespaceId", "DELETE", ([_url, accountId, namespaceId]) => { @@ -312,18 +330,23 @@ describe("wrangler", () => { expirationTtl?: number ) { const requests = { count: 0 }; - setMock( - "/accounts/:accountId/storage/kv/namespaces/:namespaceId/values/:key\\?:query", + setMockResponse( + "/accounts/:accountId/storage/kv/namespaces/:namespaceId/values/:key", "PUT", - ([_url, accountId, namespaceId, key, query], { body }) => { + ([_url, accountId, namespaceId, key], { body }, query) => { requests.count++; expect(accountId).toEqual("some-account-id"); expect(namespaceId).toEqual(expectedNamespaceId); expect(key).toEqual(expectedKey); expect(body).toEqual(expectedValue); - expect(query).toContain(`expiration=${expiration ?? ""}`); - expect(query).toContain(`expiration_ttl=${expirationTtl ?? ""}`); - return null; + expect(query.get("expiration")).toEqual(`${expiration}`); + expect(query.get("expiration_ttl")).toEqual(`${expirationTtl}`); + return { + success: true, + result: null, + errors: [], + messages: [], + }; } ); return requests; @@ -660,7 +683,7 @@ describe("wrangler", () => { expectedValue: string ) { const requests = { count: 0 }; - setMock( + setMockRawResponse( "/accounts/:accountId/storage/kv/namespaces/:namespaceId/values/:key", ([_url, accountId, namespaceId, key]) => { requests.count++; diff --git a/packages/wrangler/src/__tests__/mock-cfetch.ts b/packages/wrangler/src/__tests__/mock-cfetch.ts index d85435d75645e..af4862f664098 100644 --- a/packages/wrangler/src/__tests__/mock-cfetch.ts +++ b/packages/wrangler/src/__tests__/mock-cfetch.ts @@ -1,47 +1,84 @@ -// This file mocks ../cfetch.ts -// so we can insert whatever responses we want from it +import type { RequestInit } from "node-fetch"; +import type { URLSearchParams } from "node:url"; import { pathToRegexp } from "path-to-regexp"; +import { CF_API_BASE_URL } from "../cfetch"; +import type { FetchResult } from "../cfetch"; + +/** + * The signature of the function that will handle a mock request. + */ +export type MockHandler = ( + uri: RegExpExecArray, + init?: RequestInit, + queryParams?: URLSearchParams +) => ResponseType; -// Sadly we cannot give use the correct `RequestInit` type from node-fetch. -// Jest needs to transform the code as part of the module mocking, and it doesn't know how to cope with such types. -type RequestInit = { method: string; body: string }; -type MockHandler = (uri: RegExpExecArray, init?: RequestInit) => unknown; -type MockFetch = { - regexp: RegExp; - method: string | undefined; - handler: MockHandler; -}; type RemoveMockFn = () => void; -let mocks: MockFetch[] = []; +interface MockFetch { + regexp: RegExp; + method: string | undefined; + handler: MockHandler; +} +const mocks: MockFetch[] = []; -export default function mockCfetch( +/** + * The mock implementation of `cfApi.fetch()`. + * + * This function will attempt to match the given request to one of the mock handlers configured by calls to `setMock()`. + * + * Once found the handler will be used to generate a mock response. + */ +export async function mockFetchInternal( resource: string, - init: RequestInit -): unknown { + init: RequestInit = {}, + queryParams?: URLSearchParams +) { for (const { regexp, method, handler } of mocks) { - const uri = regexp.exec(resource); + const resourcePath = new URL(resource, CF_API_BASE_URL).pathname; + const uri = regexp.exec(resourcePath); // Do the resource path and (if specified) the HTTP method match? if (uri !== null && (!method || method === init.method)) { // The `resource` regular expression will extract the labelled groups from the URL. // These are passed through to the `handler` call, to allow it to do additional checks or behaviour. - return handler(uri, init); // TODO: should we have some kind of fallthrough system? we'll see. + return handler(uri, init, queryParams); // TODO: should we have some kind of fallthrough system? we'll see. } } throw new Error(`no mocks found for ${init.method}: ${resource}`); } /** - * Specify an expected resource path that is to be handled. + * Specify an expected resource path that is to be handled, resulting in a raw JSON response. + * + * @param resource The path of the resource to be matched. + * This can include wildcards whose value will be passed to the `handler`. + * @param handler The function that will generate the mock response for this request. */ -export function setMock( +export function setMockRawResponse( + resource: string, + handler: MockHandler +): RemoveMockFn; +/** + * Specify an expected resource path that is to be handled, resulting in a raw JSON response. + * + * @param resource The path of the resource to be matched. + * This can include wildcards whose value will be passed to the `handler`. + * @param method The HTTP method (e.g. GET, POST, etc) that the request must have to match this mock handler. + * @param handler The function that will generate the mock response for this request. + */ +export function setMockRawResponse( resource: string, method: string, - handler: MockHandler + handler: MockHandler ): RemoveMockFn; -export function setMock(resource: string, handler: MockHandler): RemoveMockFn; -export function setMock(resource: string, ...args: unknown[]): RemoveMockFn { - const handler = args.pop() as MockHandler; +/** + * Specify an expected resource path that is to be handled, resulting in a raw JSON response. + */ +export function setMockRawResponse( + resource: string, + ...args: [string, MockHandler] | [MockHandler] +): RemoveMockFn { + const handler = args.pop() as MockHandler; const method = args.pop() as string; const mock = { resource, @@ -51,13 +88,82 @@ export function setMock(resource: string, ...args: unknown[]): RemoveMockFn { }; mocks.push(mock); return () => { - mocks = mocks.filter((x) => x !== mock); + const mockIndex = mocks.indexOf(mock); + if (mockIndex !== -1) { + mocks.splice(mockIndex, 1); + } }; } -export function unsetAllMocks() { - mocks = []; +/** + * Specify an expected resource path that is to be handled, resulting in a FetchRequest. + * + * @param resource The path of the resource to be matched. + * This can include wildcards whose value will be passed to the `handler`. + * @param handler The function that will generate the mock response for this request. + */ +export function setMockResponse( + resource: string, + handler: MockHandler> +): RemoveMockFn; +/** + * Specify an expected resource path that is to be handled, resulting in a FetchRequest.. + * + * @param resource The path of the resource to be matched. + * This can include wildcards whose value will be passed to the `handler`. + * @param method The HTTP method (e.g. GET, POST, etc) that the request must have to match this mock handler. + * @param handler The function that will generate the mock response for this request. + */ +export function setMockResponse( + resource: string, + method: string, + handler: MockHandler> +): RemoveMockFn; +/** + * Specify an expected resource path that is to be handled, resulting in a FetchRequest. + */ +export function setMockResponse( + resource: string, + ...args: + | [string, MockHandler>] + | [MockHandler>] +): RemoveMockFn { + const handler = args.pop() as MockHandler>; + const method = args.pop() as string; + return setMockRawResponse(resource, method, handler); +} + +/** + * A helper to make it easier to create `FetchResult` objects in tests. + */ +export function createFetchResult( + result: ResponseType, + success = true, + errors = [], + messages = [], + result_info?: unknown +): FetchResult { + return result_info + ? { + result, + success, + errors, + messages, + result_info, + } + : { + result, + success, + errors, + messages, + }; } -export const CF_API_BASE_URL = - process.env.CF_API_BASE_URL || "https://api.cloudflare.com/client/v4"; +/** + * Remove all the configured mock handlers. + * + * This should be called in an `afterEach()` block to ensure that mock handlers do not leak between tests. + */ +export function unsetAllMocks() { + mocks.length = 0; +} diff --git a/packages/wrangler/src/api/preview.ts b/packages/wrangler/src/api/preview.ts index e0f02d8617b94..5300bb20280c9 100644 --- a/packages/wrangler/src/api/preview.ts +++ b/packages/wrangler/src/api/preview.ts @@ -1,7 +1,7 @@ -import cfetch from "../cfetch"; +import fetch from "node-fetch"; +import { fetchResult } from "../cfetch"; import { toFormData } from "./form_data"; import type { CfAccount, CfWorkerInit } from "./worker"; -import fetch from "node-fetch"; /** * A preview mode. @@ -60,7 +60,7 @@ async function sessionToken(account: CfAccount): Promise { ? `/zones/${zoneId}/workers/edge-preview` : `/accounts/${accountId}/workers/subdomain/edge-preview`; - const { exchange_url } = await cfetch<{ exchange_url: string }>(initUrl); + const { exchange_url } = await fetchResult<{ exchange_url: string }>(initUrl); const { inspector_websocket, token } = (await ( await fetch(exchange_url) ).json()) as { inspector_websocket: string; token: string }; @@ -106,7 +106,7 @@ export async function previewToken( const formData = toFormData(worker); formData.set("wrangler-session-config", JSON.stringify(mode)); - const { preview_token } = await cfetch<{ preview_token: string }>(url, { + const { preview_token } = await fetchResult<{ preview_token: string }>(url, { method: "POST", // @ts-expect-error TODO: fix this body: formData, diff --git a/packages/wrangler/src/cfetch.ts b/packages/wrangler/src/cfetch.ts deleted file mode 100644 index 644d13c473eda..0000000000000 --- a/packages/wrangler/src/cfetch.ts +++ /dev/null @@ -1,72 +0,0 @@ -import fetch from "node-fetch"; -import type { RequestInit } from "node-fetch"; -import { getAPIToken } from "./user"; -import { loginOrRefreshIfRequired } from "./user"; - -export const CF_API_BASE_URL = - process.env.CF_API_BASE_URL || "https://api.cloudflare.com/client/v4"; - -export default async function fetchWithAuthAndLoginIfRequired( - resource: string, - init: RequestInit = {} -): Promise { - const loggedIn = await loginOrRefreshIfRequired(); - if (!loggedIn) { - throw new Error("Not logged in"); - } - const apiToken = getAPIToken(); - if (!apiToken) { - throw new Error("No API token found"); - } - // @ts-expect-error Authorization isn't non HeadersInit, annoyingly - if (init.headers?.Authorization) { - throw new Error( - "fetchWithAuthAndLoginIfRequired will not add an authorisation header a request that already specifies it" - ); - } - // should I bother with response code? - // maybe I can just throw the json error? - - const response = await fetch(`${CF_API_BASE_URL}${resource}`, { - method: "GET", - ...init, - headers: { - ...(init.headers || {}), - Authorization: `Bearer ${apiToken}`, - }, - }); - const text = await response.text(); - let json; - try { - json = JSON.parse(text); - } catch (parseError) { - // hate this edge case - // the only api call I know that doesn't return json is kv:key get - - // maybe it's a plain response - if (response.ok) { - // @ts-expect-error bleh - return text; - } else { - // UGH. - throw new Error(`${response.status}: ${response.statusText}`); - } - } - - if (json.success) { - return json.result; - } else { - const errorDesc = json.errors?.[0]; - if (errorDesc) { - // TODO: map .message to real human readable strings - const error = new Error(`${errorDesc.code}: ${errorDesc.message}`); - // @ts-expect-error hacksss - error.code = errorDesc.code; - throw error; - } else { - // This should almost never happen. - // ... which means it'll probably happen, lol. - throw new Error(`${response.status}: ${response.statusText}`); - } - } -} diff --git a/packages/wrangler/src/cfetch/index.ts b/packages/wrangler/src/cfetch/index.ts new file mode 100644 index 0000000000000..14a2cef30841f --- /dev/null +++ b/packages/wrangler/src/cfetch/index.ts @@ -0,0 +1,103 @@ +import type { RequestInit } from "node-fetch"; +import { URLSearchParams } from "url"; +import { fetchInternal } from "./internal"; + +// Check out https://api.cloudflare.com/ for API docs. + +export { CF_API_BASE_URL } from "./internal"; + +export interface FetchError { + code: number; + message: string; +} +export interface FetchResult { + success: boolean; + result: ResponseType; + errors: FetchError[]; + messages: string[]; + result_info?: unknown; +} + +/** + * Make a fetch request for a raw JSON value. + */ +export async function fetchRaw( + resource: string, + init: RequestInit = {}, + queryParams?: URLSearchParams +): Promise { + return fetchInternal(resource, init, queryParams); +} + +/** + * Make a fetch request, and extract the `result` from the JSON response. + */ +export async function fetchResult( + resource: string, + init: RequestInit = {}, + queryParams?: URLSearchParams +): Promise { + const json = await fetchInternal>( + resource, + init, + queryParams + ); + if (json.success) { + return json.result; + } else { + throwFetchError(resource, json); + } +} + +/** + * Make a fetch request for a list of values, + * extracting the `result` from the JSON response, + * and repeating the request if the results are paginated. + */ +export async function fetchListResult( + resource: string, + init: RequestInit = {}, + queryParams?: URLSearchParams +): Promise { + const results: ResponseType[] = []; + let getMoreResults = true; + let cursor: string | undefined; + while (getMoreResults) { + if (cursor) { + queryParams = new URLSearchParams(queryParams); + queryParams.append("cursor", cursor); + } + const json = await fetchInternal>( + resource, + init, + queryParams + ); + if (json.success) { + results.push(...json.result); + if (hasCursor(json.result_info)) { + cursor = json.result_info?.cursor; + } else { + getMoreResults = false; + } + } else { + throwFetchError(resource, json); + } + } + return results; +} + +function throwFetchError( + resource: string, + response: FetchResult +): never { + response.messages.forEach((message) => console.warn(message)); + const errors = response.errors + .map((error) => `${error.code}: ${error.message}`) + .join("\n"); + throw new Error(`Failed to fetch ${resource} - ${errors}`); +} + +function hasCursor(result_info: unknown): result_info is { cursor: string } { + if (!result_info) return false; + return "cursor" in (result_info as object); +} diff --git a/packages/wrangler/src/cfetch/internal.ts b/packages/wrangler/src/cfetch/internal.ts new file mode 100644 index 0000000000000..d77163e404c9c --- /dev/null +++ b/packages/wrangler/src/cfetch/internal.ts @@ -0,0 +1,69 @@ +import fetch from "node-fetch"; +import type { RequestInit, HeadersInit } from "node-fetch"; +import { getAPIToken, loginOrRefreshIfRequired } from "../user"; + +export const CF_API_BASE_URL = + process.env.CF_API_BASE_URL || "https://api.cloudflare.com/client/v4"; + +/** + * Make a fetch request to the Cloudflare API. + * + * This function handles acquiring the API token and logging the caller in, as necessary. + * + * Check out https://api.cloudflare.com/ for API docs. + * + * This function should not be used directly, instead use the functions in `cfetch/index.ts`. + */ +export async function fetchInternal( + resource: string, + init: RequestInit = {}, + queryParams?: URLSearchParams +): Promise { + await requireLoggedIn(); + const apiToken = requireApiToken(); + const headers = cloneHeaders(init.headers); + addAuthorizationHeader(headers, apiToken); + + const queryString = queryParams ? `?${queryParams.toString()}` : ""; + const response = await fetch(`${CF_API_BASE_URL}${resource}${queryString}`, { + method: "GET", + ...init, + headers, + }); + + if (response.ok) { + return (await response.json()) as ResponseType; + } else { + throw new Error( + `Failed to fetch ${resource} - ${response.status}: ${response.statusText}` + ); + } +} + +async function requireLoggedIn(): Promise { + const loggedIn = await loginOrRefreshIfRequired(); + if (!loggedIn) { + throw new Error("Not logged in."); + } +} + +function requireApiToken(): string { + const apiToken = getAPIToken(); + if (!apiToken) { + throw new Error("No API token found."); + } + return apiToken; +} + +function cloneHeaders(headers: HeadersInit): HeadersInit { + return { ...headers }; +} + +function addAuthorizationHeader(headers: HeadersInit, apiToken: string): void { + if (headers["Authorization"]) { + throw new Error( + "The request already specifies an authorisation header - cannot add a new one." + ); + } + headers["Authorization"] = `Bearer ${apiToken}`; +} diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index f991bf65c5923..e0b4469f54d80 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -30,7 +30,7 @@ import { import { pages } from "./pages"; -import cfetch from "./cfetch"; +import { fetchResult, fetchRaw } from "./cfetch"; import publish from "./publish"; import path from "path/posix"; @@ -537,7 +537,7 @@ export async function main(argv: string[]): Promise { // `preview_id` instead of `id` so that they don't // break production data. So here we check that a `preview_id` // has actually been configured. - // This whole block of code will be obsolted in the future + // This whole block of code will be obsoleted in the future // when we have copy-on-write for previews on edge workers. if (!preview_id) { // TODO: This error has to be a _lot_ better, ideally just asking @@ -836,7 +836,7 @@ export async function main(argv: string[]): Promise { throw new Error("missing zone id"); } - console.log(await cfetch(`/zones/${zone}/workers/routes`)); + console.log(await fetchResult(`/zones/${zone}/workers/routes`)); } ) .command( @@ -866,7 +866,7 @@ export async function main(argv: string[]): Promise { } console.log( - await cfetch(`/zones/${zone}/workers/routes/${args.id}`, { + await fetchResult(`/zones/${zone}/workers/routes/${args.id}`, { method: "DELETE", }) ); @@ -953,7 +953,7 @@ export async function main(argv: string[]): Promise { "password" ); async function submitSecret() { - return await cfetch( + return await fetchResult( `/accounts/${config.account_id}/workers/scripts/${scriptName}/secrets/`, { method: "PUT", @@ -972,7 +972,7 @@ export async function main(argv: string[]): Promise { } catch (e) { if (e.code === 10007) { // upload a draft worker - await cfetch( + await fetchResult( `/accounts/${config.account_id}/workers/scripts/${scriptName}`, { method: "PUT", @@ -1058,7 +1058,7 @@ export async function main(argv: string[]): Promise { ); console.log( - await cfetch( + await fetchResult( `/accounts/${config.account_id}/workers/scripts/${scriptName}/secrets/${args.key}`, { method: "DELETE" } ) @@ -1115,7 +1115,7 @@ export async function main(argv: string[]): Promise { // -- snip, end -- console.log( - await cfetch( + await fetchResult( `/accounts/${config.account_id}/workers/scripts/${scriptName}/secrets` ) ); @@ -1333,7 +1333,7 @@ export async function main(argv: string[]): Promise { // -- snip, end -- - await cfetch<{ id: string }>( + await fetchResult<{ id: string }>( `/accounts/${config.account_id}/storage/kv/namespaces/${id}`, { method: "DELETE" } ); @@ -1595,12 +1595,8 @@ export async function main(argv: string[]): Promise { // -- snip, end -- - // annoyingly, the API for this one doesn't return the - // data in the 'standard' format. goddammit. - // That's why we have the fallthrough response in cfetch. - // Oh well. console.log( - await cfetch( + await fetchRaw( `/accounts/${config.account_id}/storage/kv/namespaces/${namespaceId}/values/${key}` ) ); @@ -1674,7 +1670,7 @@ export async function main(argv: string[]): Promise { // -- snip, end -- - await cfetch( + await fetchResult( `/accounts/${config.account_id}/storage/kv/namespaces/${namespaceId}/values/${key}`, { method: "DELETE" } ); diff --git a/packages/wrangler/src/kv.tsx b/packages/wrangler/src/kv.tsx index 5a753d92c2601..560138cba3b07 100644 --- a/packages/wrangler/src/kv.tsx +++ b/packages/wrangler/src/kv.tsx @@ -1,6 +1,6 @@ +import { URLSearchParams } from "node:url"; import type { Config } from "./config"; -import cfetch from "./cfetch"; -import qs from "node:querystring"; +import { fetchListResult, fetchResult } from "./cfetch"; type KvArgs = { binding?: string; @@ -19,7 +19,7 @@ export async function createNamespace( accountId: string, title: string ): Promise { - const response = await cfetch<{ id: string }>( + const response = await fetchResult<{ id: string }>( `/accounts/${accountId}/storage/kv/namespaces`, { method: "POST", @@ -54,8 +54,15 @@ export async function listNamespaces( let page = 1; const results: KVNamespaceInfo[] = []; while (results.length % pageSize === 0) { - const json = await cfetch( - `/accounts/${accountId}/storage/kv/namespaces?per_page=${pageSize}&order=title&direction=asc&page=${page}` + const json = await fetchResult( + `/accounts/${accountId}/storage/kv/namespaces`, + {}, + new URLSearchParams({ + per_page: pageSize.toString(), + order: "title", + direction: "asc", + page: page.toString(), + }) ); page++; results.push(...json); @@ -66,19 +73,22 @@ export async function listNamespaces( return results; } +export interface NamespaceKeyInfo { + name: string; + expiration?: number; + metadata?: { [key: string]: unknown }; +} + export async function listNamespaceKeys( accountId: string, namespaceId: string, prefix?: string, limit?: number ) { - // TODO: this doesn't appear to do pagination - return await cfetch< - { name: string; expiration: number; metadata: { [key: string]: unknown } }[] - >( - `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/keys?${qs.stringify( - { prefix, limit } - )}` + return await fetchListResult( + `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/keys`, + {}, + new URLSearchParams({ prefix, limit: limit.toString() }) ); } @@ -89,16 +99,15 @@ export async function putKeyValue( value: string, args?: { expiration?: number; expiration_ttl?: number } ) { - return await cfetch( - `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/values/${key}?${ - args - ? qs.stringify({ - expiration: args.expiration, - expiration_ttl: args.expiration_ttl, - }) - : "" - }`, - { method: "PUT", body: value } + return await fetchResult( + `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/values/${key}`, + { method: "PUT", body: value }, + args + ? new URLSearchParams({ + expiration: args.expiration?.toString(), + expiration_ttl: args.expiration_ttl?.toString(), + }) + : undefined ); } @@ -107,7 +116,7 @@ export async function putBulkKeyValue( namespaceId: string, keyvalueStr: string ) { - return await cfetch( + return await fetchResult( `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/bulk`, { method: "PUT", @@ -122,7 +131,7 @@ export async function deleteBulkKeyValue( namespaceId: string, keyStr: string ) { - return await cfetch( + return await fetchResult( `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/bulk`, { method: "DELETE", diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index e80f40a4cc2c8..9f8647becaf5a 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -1,15 +1,15 @@ -import type { CfWorkerInit } from "./api/worker"; -import { toFormData } from "./api/form_data"; +import assert from "node:assert"; +import path from "node:path"; +import { readFile } from "node:fs/promises"; import esbuild from "esbuild"; +import { execa } from "execa"; import tmp from "tmp-promise"; +import type { CfWorkerInit } from "./api/worker"; +import { toFormData } from "./api/form_data"; +import { fetchResult } from "./cfetch"; import type { Config } from "./config"; -import path from "path"; -import { readFile } from "fs/promises"; -import cfetch from "./cfetch"; -import assert from "node:assert"; -import { syncAssets } from "./sites"; import makeModuleCollector from "./module-collection"; -import { execa } from "execa"; +import { syncAssets } from "./sites"; type CfScriptFormat = void | "modules" | "service-worker"; @@ -155,7 +155,7 @@ export default async function publish(props: Props): Promise { // get current migration tag let migrations; if ("migrations" in config) { - const scripts = await cfetch<{ id: string; migration_tag: string }[]>( + const scripts = await fetchResult<{ id: string; migration_tag: string }[]>( `/accounts/${accountId}/workers/scripts` ); const script = scripts.find(({ id }) => id === scriptName); @@ -246,8 +246,8 @@ export default async function publish(props: Props): Promise { ? `/accounts/${accountId}/workers/services/${scriptName}/environments/${envName}` : `/accounts/${accountId}/workers/scripts/${scriptName}`; - // Upload the script so it has time to propogate. - const { available_on_subdomain } = await cfetch( + // Upload the script so it has time to propagate. + const { available_on_subdomain } = await fetchResult( `${workerUrl}?available_on_subdomain=true`, { method: "PUT", @@ -261,7 +261,7 @@ export default async function publish(props: Props): Promise { const deployments: Promise[] = []; const userSubdomain = ( - await cfetch<{ subdomain: string }>( + await fetchResult<{ subdomain: string }>( `/accounts/${accountId}/workers/subdomain` ) ).subdomain; @@ -275,7 +275,7 @@ export default async function publish(props: Props): Promise { // TODO: Make this configurable. if (!available_on_subdomain) { deployments.push( - cfetch(`${workerUrl}/subdomain`, { + fetchResult(`${workerUrl}/subdomain`, { method: "POST", body: JSON.stringify({ enabled: true }), headers: { @@ -299,7 +299,7 @@ export default async function publish(props: Props): Promise { // Update routing table for the script. if (routes && routes.length) { deployments.push( - cfetch(`${workerUrl}/routes`, { + fetchResult(`${workerUrl}/routes`, { // TODO: PATCH will not delete previous routes on this script, // whereas PUT will. We need to decide on the default behaviour // and how to configure it. @@ -324,7 +324,7 @@ export default async function publish(props: Props): Promise { // TODO: rename this to `schedules`? if (triggers && triggers.length) { deployments.push( - cfetch(`${workerUrl}/schedules`, { + fetchResult(`${workerUrl}/schedules`, { // TODO: Unlike routes, this endpoint does not support PATCH. // So technically, this will override any previous schedules. // We should change the endpoint to support PATCH. diff --git a/packages/wrangler/src/sites.tsx b/packages/wrangler/src/sites.tsx index c81f2ffc31df3..1d9a00d487712 100644 --- a/packages/wrangler/src/sites.tsx +++ b/packages/wrangler/src/sites.tsx @@ -1,11 +1,10 @@ -import { readdir, readFile } from "node:fs/promises"; +import crypto from "node:crypto"; import { createReadStream } from "node:fs"; -import cfetch from "./cfetch"; +import * as path from "node:path"; +import { readdir, readFile } from "node:fs/promises"; +import { fetchResult } from "./cfetch"; import { listNamespaceKeys, listNamespaces, putBulkKeyValue } from "./kv"; -import * as path from "path"; -import crypto from "node:crypto"; - async function* getFilesInFolder(dirPath: string): AsyncIterable { const files = await readdir(dirPath, { withFileTypes: true }); for (const file of files) { @@ -54,7 +53,7 @@ async function createKVNamespaceIfNotAlreadyExisting( // else we make the namespace // TODO: use an export from ./kv - const json = await cfetch<{ id: string }>( + const json = await fetchResult<{ id: string }>( `/accounts/${accountId}/storage/kv/namespaces`, { method: "POST", diff --git a/packages/wrangler/src/tail.tsx b/packages/wrangler/src/tail.tsx index 06f5a6b15b16c..aca4e8d2aed52 100644 --- a/packages/wrangler/src/tail.tsx +++ b/packages/wrangler/src/tail.tsx @@ -1,6 +1,6 @@ import WebSocket from "ws"; -import cfetch from "./cfetch"; import { version as packageVersion } from "../package.json"; +import { fetchResult } from "./cfetch"; export type TailApiResponse = { id: string; @@ -27,7 +27,9 @@ async function createTailButDontConnect( ): Promise { const createTailUrl = makeCreateTailUrl(accountId, workerName); /// https://api.cloudflare.com/#worker-tail-logs-start-tail - return await cfetch(createTailUrl, { method: "POST" }); + return await fetchResult(createTailUrl, { + method: "POST", + }); } export async function createTail( @@ -48,7 +50,7 @@ export async function createTail( // deletes the tail async function deleteTail() { - await cfetch(deleteUrl, { method: "DELETE" }); + await fetchResult(deleteUrl, { method: "DELETE" }); } const tail = new WebSocket(websocketUrl, "trace-v1", {