diff --git a/.changeset/modern-books-leave.md b/.changeset/modern-books-leave.md new file mode 100644 index 000000000000..af7a99c9e31a --- /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 000000000000..f20def44ed0f --- /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 1c51c5a52f3b..6575a481e2ff 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 4248f41ed437..88af107985f8 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/packages/wrangler/src/__tests__/kv.test.ts @@ -1,7 +1,12 @@ -import { setMock, unsetAllMocks } from "./mock-cfetch"; +import { writeFileSync } from "fs"; +import { + setMockResponse, + setMockRawResponse, + unsetAllMocks, + createFetchResult, +} from "./mock-cfetch"; import { runWrangler } from "./run-wrangler"; import { runInTempDir } from "./run-in-tmp"; -import { writeFileSync } from "fs"; describe("wrangler", () => { runInTempDir(); @@ -13,12 +18,12 @@ 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" }; } @@ -172,18 +177,18 @@ 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]); + 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 namespaces.slice((page - 1) * pageSize, page * pageSize); } ); @@ -196,13 +201,17 @@ 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); }); it("should make multiple requests for paginated results", async () => { - // Create a lot of mock namespaces, so that the cfetch requests will be paginated + // Create a lot of mock namespaces, so that the fetch requests will be paginated const KVNamespaces = []; for (let i = 0; i < 550; i++) { KVNamespaces.push({ title: "title-" + i, id: "id-" + i }); @@ -218,7 +227,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]) => { @@ -242,7 +251,9 @@ describe("wrangler", () => { it("should delete a namespace specified by binding name", async () => { writeWranglerConfig(); const requests = mockDeleteRequest("bound-id"); - await runWrangler(`kv:namespace delete --binding someBinding`); + await runWrangler( + `kv:namespace delete --binding someBinding --preview false` + ); expect(requests.count).toEqual(1); }); @@ -285,9 +296,12 @@ describe("wrangler", () => { it("should delete a namespace specified by binding name in a given environment", async () => { writeWranglerConfig(); const requests = mockDeleteRequest("env-bound-id"); - await runWrangler( - `kv:namespace delete --binding someBinding --env some-environment` + const { stdout, stderr, error } = await runWrangler( + `kv:namespace delete --binding someBinding --env some-environment --preview false` ); + expect(stdout).toMatchInlineSnapshot(`""`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(error).toMatchInlineSnapshot(`undefined`); expect(requests.count).toEqual(1); }); @@ -312,17 +326,17 @@ 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 ?? ""}`); + expect(query.get("expiration")).toEqual(`${expiration}`); + expect(query.get("expiration_ttl")).toEqual(`${expirationTtl}`); return null; } ); @@ -623,10 +637,10 @@ describe("wrangler", () => { ); expect(stdout).toMatchInlineSnapshot(`""`); expect(stderr).toMatchInlineSnapshot( - `"No KV Namespaces found with binding otherBinding!"` + `"A namespace with binding name \\"otherBinding\\" was not found in the configured \\"kv_namespaces\\"."` ); expect(error).toMatchInlineSnapshot( - `[Error: No KV Namespaces found with binding otherBinding!]` + `[Error: A namespace with binding name "otherBinding" was not found in the configured "kv_namespaces".]` ); }); @@ -651,7 +665,148 @@ describe("wrangler", () => { }); }); - describe("list", () => {}); + describe("list", () => { + function mockKeyListRequest( + expectedNamespaceId: string, + expectedKeys: string[], + keysPerRequest = 1000 + ) { + const requests = { count: 0 }; + setMockRawResponse( + "/accounts/:accountId/storage/kv/namespaces/:namespaceId/keys", + ([_url, accountId, namespaceId], _init, query) => { + requests.count++; + expect(accountId).toEqual("some-account-id"); + expect(namespaceId).toEqual(expectedNamespaceId); + if (expectedKeys.length <= keysPerRequest) { + return createFetchResult(expectedKeys); + } else { + const start = parseInt(query.get("cursor")) || 0; + const end = start + keysPerRequest; + const cursor = end < expectedKeys.length ? end : undefined; + return createFetchResult( + expectedKeys.slice(start, end), + true, + [], + [], + { cursor } + ); + } + } + ); + return requests; + } + + it("should list the keys of a namespace specified by namespace-id", async () => { + const keys = ["key-1", "key-2", "key-3"]; + mockKeyListRequest("some-namespace-id", keys); + const { error, stdout, stderr } = await runWrangler( + "kv:key list --namespace-id some-namespace-id" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(stdout).toMatchInlineSnapshot(` + "key-1 + key-2 + key-3" + `); + }); + + it("should list the keys of a namespace specified by binding", async () => { + writeWranglerConfig(); + const keys = ["key-1", "key-2", "key-3"]; + mockKeyListRequest("bound-id", keys); + const { error, stdout, stderr } = await runWrangler( + "kv:key list --binding someBinding" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(stdout).toMatchInlineSnapshot(` + "key-1 + key-2 + key-3" + `); + }); + + it("should list the keys of a preview namespace specified by binding", async () => { + writeWranglerConfig(); + const keys = ["key-1", "key-2", "key-3"]; + mockKeyListRequest("preview-bound-id", keys); + const { error, stdout, stderr } = await runWrangler( + "kv:key list --binding someBinding --preview" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(stdout).toMatchInlineSnapshot(` + "key-1 + key-2 + key-3" + `); + }); + + it("should list the keys of a namespace specified by binding, in a given environment", async () => { + writeWranglerConfig(); + const keys = ["key-1", "key-2", "key-3"]; + mockKeyListRequest("env-bound-id", keys); + const { error, stdout, stderr } = await runWrangler( + "kv:key list --binding someBinding --env some-environment" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(stdout).toMatchInlineSnapshot(` + "key-1 + key-2 + key-3" + `); + }); + + it("should list the keys of a preview namespace specified by binding, in a given environment", async () => { + writeWranglerConfig(); + const keys = ["key-1", "key-2", "key-3"]; + mockKeyListRequest("preview-env-bound-id", keys); + const { error, stdout, stderr } = await runWrangler( + "kv:key list --binding someBinding --preview --env some-environment" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(stdout).toMatchInlineSnapshot(` + "key-1 + key-2 + key-3" + `); + }); + + it("should make multiple requests for paginated results", async () => { + // Create a lot of mock keys, so that the fetch requests will be paginated + const keys = []; + for (let i = 0; i < 550; i++) { + keys.push("key-" + i); + } + // Ask for the keys in pages of size 100. + const requests = mockKeyListRequest("some-namespace-id", keys, 100); + const { stdout, stderr, error } = await runWrangler( + "kv:key list --namespace-id some-namespace-id --limit 100" + ); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(stdout).toEqual(keys.join("\n")); + expect(requests.count).toEqual(6); + }); + + it("should error if a given binding name is not in the configured kv namespaces", async () => { + writeWranglerConfig(); + const { error, stdout, stderr } = await runWrangler( + "kv:key list --binding otherBinding" + ); + expect(error).toMatchInlineSnapshot( + `[Error: A namespace with binding name "otherBinding" was not found in the configured "kv_namespaces".]` + ); + expect(stderr).toMatchInlineSnapshot( + `"A namespace with binding name \\"otherBinding\\" was not found in the configured \\"kv_namespaces\\"."` + ); + expect(stdout).toMatchInlineSnapshot(`""`); + }); + }); describe("get", () => { function mockKeyGetRequest( @@ -660,7 +815,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++; @@ -754,7 +909,7 @@ describe("wrangler", () => { --binding The name of the namespace to get from [string] --namespace-id The id of the namespace to get from [string] --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] + --preview Interact with a preview namespace [boolean] [default: false] Not enough non-option arguments: got 0, need at least 1" `); @@ -785,7 +940,7 @@ describe("wrangler", () => { --binding The name of the namespace to get from [string] --namespace-id The id of the namespace to get from [string] --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] + --preview Interact with a preview namespace [boolean] [default: false] Exactly one of the arguments binding and namespace-id is required" `); @@ -818,7 +973,7 @@ describe("wrangler", () => { --binding The name of the namespace to get from [string] --namespace-id The id of the namespace to get from [string] --env Perform on a specific environment [string] - --preview Interact with a preview namespace [boolean] + --preview Interact with a preview namespace [boolean] [default: false] Arguments binding and namespace-id are mutually exclusive" `); @@ -834,29 +989,93 @@ describe("wrangler", () => { ); expect(stdout).toMatchInlineSnapshot(`""`); expect(stderr).toMatchInlineSnapshot( - `"No KV Namespaces found with binding otherBinding!"` + `"A namespace with binding name \\"otherBinding\\" was not found in the configured \\"kv_namespaces\\"."` ); expect(error).toMatchInlineSnapshot( - `[Error: No KV Namespaces found with binding otherBinding!]` + `[Error: A namespace with binding name "otherBinding" was not found in the configured "kv_namespaces".]` ); }); + }); - it("should error if a given binding has both preview and non-preview and --preview is not specified", async () => { + describe("delete", () => { + function mockDeleteRequest( + expectedNamespaceId: string, + expectedKey: string + ) { + const requests = { count: 0 }; + setMockResponse( + "/accounts/:accountId/storage/kv/namespaces/:namespaceId/values/:key", + "DELETE", + ([_url, accountId, namespaceId, key]) => { + requests.count++; + expect(accountId).toEqual("some-account-id"); + expect(namespaceId).toEqual(expectedNamespaceId); + expect(key).toEqual(expectedKey); + return null; + } + ); + return requests; + } + + it("should delete a key in a namespace specified by id", async () => { + const requests = mockDeleteRequest("some-namespace-id", "someKey"); + await runWrangler( + `kv:key delete --namespace-id some-namespace-id someKey` + ); + expect(requests.count).toEqual(1); + }); + + it("should delete a key in a namespace specified by binding name", async () => { writeWranglerConfig(); - const { error, stderr, stdout } = await runWrangler( - "kv:key get my-key --binding someBinding" + const requests = mockDeleteRequest("bound-id", "someKey"); + await runWrangler( + `kv:key delete --binding someBinding --preview false someKey` + ); + expect(requests.count).toEqual(1); + }); + + it("should delete a key in a preview namespace specified by binding name", async () => { + writeWranglerConfig(); + const requests = mockDeleteRequest("preview-bound-id", "someKey"); + await runWrangler( + `kv:key delete --binding someBinding --preview someKey` + ); + expect(requests.count).toEqual(1); + }); + + it("should error if a given binding name is not in the configured kv namespaces", async () => { + writeWranglerConfig(); + const { stderr } = await runWrangler( + `kv:key delete --binding otherBinding someKey` ); - expect(stdout).toMatchInlineSnapshot(`""`); expect(stderr).toMatchInlineSnapshot( - `"someBinding has both a namespace ID and a preview ID. Specify \\"--preview\\" or \\"--preview false\\" to avoid writing data to the wrong namespace."` + `"A namespace with binding name \\"otherBinding\\" was not found in the configured \\"kv_namespaces\\"."` ); - expect(error).toMatchInlineSnapshot( - `[Error: someBinding has both a namespace ID and a preview ID. Specify "--preview" or "--preview false" to avoid writing data to the wrong namespace.]` + }); + + it("should delete a key in a namespace specified by binding name in a given environment", async () => { + writeWranglerConfig(); + const requests = mockDeleteRequest("env-bound-id", "someKey"); + const { stdout, stderr, error } = await runWrangler( + `kv:key delete --binding someBinding --env some-environment --preview false someKey` ); + expect(stdout).toMatchInlineSnapshot( + `"deleting the key \\"someKey\\" on namespace env-bound-id"` + ); + expect(stderr).toMatchInlineSnapshot(`""`); + expect(error).toMatchInlineSnapshot(`undefined`); + expect(requests.count).toEqual(1); }); - }); - describe("delete", () => {}); + it("should delete a key in a preview namespace specified by binding name in a given environment", async () => { + writeWranglerConfig(); + const requests = mockDeleteRequest("preview-env-bound-id", "someKey"); + await runWrangler( + `kv:key delete --binding someBinding --env some-environment --preview someKey` + ); + expect(requests.count).toEqual(1); + }); + }); }); }); diff --git a/packages/wrangler/src/__tests__/mock-cfetch.ts b/packages/wrangler/src/__tests__/mock-cfetch.ts index d85435d75645..f0ca6c035743 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,84 @@ 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`. + * + * The mock `handler` should return the `result`, which will then be wrapped in a `FetchRequest` object. + * + * @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, (...handlerArgs) => + createFetchResult(handler(...handlerArgs)) + ); +} + +/** + * 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 e0f02d8617b9..5300bb20280c 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 644d13c473ed..000000000000 --- 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 000000000000..0734b9128837 --- /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.set("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 } { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (result_info as any)?.cursor !== undefined; +} diff --git a/packages/wrangler/src/cfetch/internal.ts b/packages/wrangler/src/cfetch/internal.ts new file mode 100644 index 000000000000..d77163e404c9 --- /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 f991bf65c592..28a116b05662 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` ) ); @@ -1298,18 +1298,12 @@ export async function main(argv: string[]): Promise { } const config = args.config as Config; - const id = - args["namespace-id"] || - (args.env - ? config.env[args.env] || {} - : config - ).kv_namespaces.find( - (namespace) => namespace.binding === args.binding - )?.[args.preview ? "preview_id" : "id"]; - if (!id) { + let id; + try { + id = getNamespaceId(args); + } catch (e) { throw new CommandLineArgsError( - "Not able to delete namespace.\n" + - `A namespace with binding name "${args.binding}" was not found in the configured "kv_namespaces".` + "Not able to delete namespace.\n" + e.message ); } @@ -1333,7 +1327,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" } ); @@ -1483,6 +1477,12 @@ export async function main(argv: string[]): Promise { type: "string", describe: "Perform on a specific environment", }) + .option("preview", { + type: "boolean", + // In the case of listing keys we will default to non-preview mode + default: false, + describe: "Interact with a preview namespace", + }) .option("prefix", { type: "string", describe: "A prefix to filter listed keys", @@ -1557,6 +1557,12 @@ export async function main(argv: string[]): Promise { .option("preview", { type: "boolean", describe: "Interact with a preview namespace", + }) + .option("preview", { + type: "boolean", + // In the case of getting key values we will default to non-preview mode + default: false, + describe: "Interact with a preview namespace", }); }, async ({ key, ...args }) => { @@ -1595,12 +1601,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 +1676,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 5a753d92c260..8b84033baf58 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,21 @@ 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 + prefix?: string ) { - // 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 }) ); } @@ -89,16 +98,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 +115,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 +130,7 @@ export async function deleteBulkKeyValue( namespaceId: string, keyStr: string ) { - return await cfetch( + return await fetchResult( `/accounts/${accountId}/storage/kv/namespaces/${namespaceId}/bulk`, { method: "DELETE", @@ -186,7 +194,7 @@ export function getNamespaceId({ // there's no KV namespaces if (!config.kv_namespaces || config.kv_namespaces.length === 0) { throw new Error( - "No KV Namespace to upload to! Either use --namespace-id to upload directly or add a KV namespace to your wrangler config file." + "No KV Namespaces configured! Either use --namespace-id to upload directly or add a KV namespace to your wrangler config file." ); } @@ -194,7 +202,9 @@ export function getNamespaceId({ // we couldn't find a namespace with that binding if (!namespace) { - throw new Error(`No KV Namespaces found with binding ${binding}!`); + throw new Error( + `A namespace with binding name "${binding}" was not found in the configured "kv_namespaces".` + ); } // end pre-flight checks diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index e80f40a4cc2c..9f8647becaf5 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 c81f2ffc31df..1d9a00d48771 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 06f5a6b15b16..aca4e8d2aed5 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", {