From ac3b44bc2ff2553b23bdfc1d65d6264138eeeb7e Mon Sep 17 00:00:00 2001 From: Sarah Shader Date: Tue, 30 Jul 2024 15:17:06 -0400 Subject: [PATCH] Change CLI deploymentFetch to insert version, authorization, and content-type headers by default (#28438) The one spot that wasn't doing this already was the network test, which uses `bareDeploymentFetch` instead. GitOrigin-RevId: f20dbbcd599a50226f2d793626db5061fd6e0147 --- npm-packages/convex/src/cli/convexExport.ts | 9 +- npm-packages/convex/src/cli/convexImport.ts | 17 +-- npm-packages/convex/src/cli/env.ts | 11 +- npm-packages/convex/src/cli/lib/config.ts | 15 +- npm-packages/convex/src/cli/lib/deploy2.ts | 19 +-- npm-packages/convex/src/cli/lib/indexes.ts | 17 +-- npm-packages/convex/src/cli/lib/logs.ts | 21 +-- npm-packages/convex/src/cli/lib/utils.ts | 149 +++++++++++++------- npm-packages/convex/src/cli/network_test.ts | 10 +- 9 files changed, 124 insertions(+), 144 deletions(-) diff --git a/npm-packages/convex/src/cli/convexExport.ts b/npm-packages/convex/src/cli/convexExport.ts index d726e75a..696507c9 100644 --- a/npm-packages/convex/src/cli/convexExport.ts +++ b/npm-packages/convex/src/cli/convexExport.ts @@ -6,7 +6,6 @@ import { deploymentFetch, logAndHandleFetchError, } from "./lib/utils.js"; -import { version } from "./version.js"; import { logFailure, oneoffContext, @@ -67,15 +66,10 @@ export const convexExport = new Command("export") : ""; showSpinner(ctx, `Creating snapshot export${deploymentNotice}`); - const fetch = deploymentFetch(deploymentUrl); - const headers = { - Authorization: `Convex ${adminKey}`, - "Convex-Client": `npm-cli-${version}`, - }; + const fetch = deploymentFetch(deploymentUrl, adminKey); try { await fetch(`/api/export/request/zip?includeStorage=${includeStorage}`, { method: "POST", - headers, }); } catch (e) { return await logAndHandleFetchError(ctx, e); @@ -124,7 +118,6 @@ export const convexExport = new Command("export") try { response = await fetch(exportUrl, { method: "GET", - headers, }); } catch (e) { return await logAndHandleFetchError(ctx, e); diff --git a/npm-packages/convex/src/cli/convexImport.ts b/npm-packages/convex/src/cli/convexImport.ts index f3c6ddd8..21f510c0 100644 --- a/npm-packages/convex/src/cli/convexImport.ts +++ b/npm-packages/convex/src/cli/convexImport.ts @@ -7,7 +7,6 @@ import { deploymentFetch, logAndHandleFetchError, } from "./lib/utils.js"; -import { version } from "./version.js"; import { logFailure, oneoffContext, @@ -143,7 +142,7 @@ export const convexImport = new Command("import") options.yes, ); } - const fetch = deploymentFetch(deploymentUrl); + const fetch = deploymentFetch(deploymentUrl, adminKey); const data = ctx.fs.createReadStream(filePath, { highWaterMark: CHUNK_SIZE, @@ -163,10 +162,6 @@ export const convexImport = new Command("import") mode, format, }; - const headers = { - Authorization: `Convex ${adminKey}`, - "Convex-Client": `npm-cli-${version}`, - }; const deploymentNotice = options.prod ? ` in your ${chalk.bold("prod")} deployment` : ""; @@ -175,7 +170,6 @@ export const convexImport = new Command("import") try { const startResp = await fetch("/api/import/start_upload", { method: "POST", - headers, }); const { uploadToken } = await startResp.json(); @@ -187,7 +181,9 @@ export const convexImport = new Command("import") uploadToken, )}&partNumber=${partNumber}`; const partResp = await fetch(partUrl, { - headers, + headers: { + "Content-Type": "application/octet-stream", + }, body: chunk, method: "POST", }); @@ -202,10 +198,6 @@ export const convexImport = new Command("import") } const finishResp = await fetch("/api/import/finish_upload", { - headers: { - ...headers, - "Content-Type": "application/json", - }, body: JSON.stringify({ import: importArgs, uploadToken, @@ -262,7 +254,6 @@ export const convexImport = new Command("import") const performUrl = `/api/perform_import`; try { await fetch(performUrl, { - headers: { ...headers, "content-type": "application/json" }, method: "POST", body: JSON.stringify({ importId }), }); diff --git a/npm-packages/convex/src/cli/env.ts b/npm-packages/convex/src/cli/env.ts index 5a910f3f..1c1baac1 100644 --- a/npm-packages/convex/src/cli/env.ts +++ b/npm-packages/convex/src/cli/env.ts @@ -20,7 +20,6 @@ import { ensureHasConvexDependency, logAndHandleFetchError, } from "./lib/utils.js"; -import { version } from "./version.js"; const envSet = new Command("set") // Pretend value is required @@ -175,17 +174,9 @@ async function callUpdateEnvironmentVariables( ctx, deploymentSelection, ); - const fetch = deploymentFetch(url); - const headers = { - Authorization: `Convex ${adminKey}`, - "Convex-Client": `npm-cli-${version}`, - }; + const fetch = deploymentFetch(url, adminKey); try { await fetch("/api/update_environment_variables", { - headers: { - ...headers, - "Content-Type": "application/json", - }, body: JSON.stringify({ changes }), method: "POST", }); diff --git a/npm-packages/convex/src/cli/lib/config.ts b/npm-packages/convex/src/cli/lib/config.ts index 8c5e43bf..12eec89c 100644 --- a/npm-packages/convex/src/cli/lib/config.ts +++ b/npm-packages/convex/src/cli/lib/config.ts @@ -561,17 +561,13 @@ export async function pullConfig( origin: string, adminKey: string, ): Promise { - const fetch = deploymentFetch(origin); + const fetch = deploymentFetch(origin, adminKey); changeSpinner(ctx, "Downloading current deployment state..."); try { const res = await fetch("/api/get_config_hashes", { method: "POST", body: JSON.stringify({ version, adminKey }), - headers: { - "Content-Type": "application/json", - "Convex-Client": `npm-cli-${version}`, - }, }); deprecationCheckWarning(ctx, res); const data = await res.json(); @@ -759,16 +755,12 @@ export async function pushConfig2( : s; console.log(JSON.stringify(serializedConfig, custom, 2)); */ - const fetch = deploymentFetch(url); + const fetch = deploymentFetch(url, adminKey); changeSpinner(ctx, "Analyzing and deploying source code..."); try { const response = await fetch("/api/deploy2/start_push", { body: JSON.stringify(serializedConfig), method: "POST", - headers: { - "Content-Type": "application/json", - "Convex-Client": `npm-cli-${version}`, - }, }); return await response.json(); } catch (error: unknown) { @@ -795,7 +787,7 @@ export async function pushConfig( pushMetrics, bundledModuleInfos, ); - const fetch = deploymentFetch(url); + const fetch = deploymentFetch(url, adminKey); try { if (config.nodeDependencies.length > 0) { changeSpinner( @@ -816,7 +808,6 @@ export async function pushConfig( headers: { "Content-Type": "application/json", "Content-Encoding": "br", - "Convex-Client": `npm-cli-${version}`, }, }); } catch (error: unknown) { diff --git a/npm-packages/convex/src/cli/lib/deploy2.ts b/npm-packages/convex/src/cli/lib/deploy2.ts index 70a91310..35323c76 100644 --- a/npm-packages/convex/src/cli/lib/deploy2.ts +++ b/npm-packages/convex/src/cli/lib/deploy2.ts @@ -1,5 +1,4 @@ import { changeSpinner, Context, logFailure } from "../../bundler/context.js"; -import { version } from "../version.js"; import { deploymentFetch, logAndHandleFetchError } from "./utils.js"; import { StartPushRequest, @@ -23,16 +22,12 @@ export async function startPush( typeof s === "string" ? s.slice(0, 40) + (s.length > 40 ? "..." : "") : s; console.log(JSON.stringify(request, custom, 2)); } - const fetch = deploymentFetch(url); + const fetch = deploymentFetch(url, request.adminKey); changeSpinner(ctx, "Analyzing and deploying source code..."); try { const response = await fetch("/api/deploy2/start_push", { body: JSON.stringify(request), method: "POST", - headers: { - "Content-Type": "application/json", - "Convex-Client": `npm-cli-${version}`, - }, }); return startPushResponse.parse(await response.json()); } catch (error: unknown) { @@ -48,7 +43,7 @@ export async function waitForSchema( url: string, startPush: StartPushResponse, ) { - const fetch = deploymentFetch(url); + const fetch = deploymentFetch(url, adminKey); try { const response = await fetch("/api/deploy2/wait_for_schema", { body: JSON.stringify({ @@ -57,10 +52,6 @@ export async function waitForSchema( dryRun: false, }), method: "POST", - headers: { - "Content-Type": "application/json", - "Convex-Client": `npm-cli-${version}`, - }, }); return await response.json(); } catch (error: unknown) { @@ -76,7 +67,7 @@ export async function finishPush( url: string, startPush: StartPushResponse, ): Promise { - const fetch = deploymentFetch(url); + const fetch = deploymentFetch(url, adminKey); changeSpinner(ctx, "Finalizing push..."); try { const response = await fetch("/api/deploy2/finish_push", { @@ -86,10 +77,6 @@ export async function finishPush( dryRun: false, }), method: "POST", - headers: { - "Content-Type": "application/json", - "Convex-Client": `npm-cli-${version}`, - }, }); return await response.json(); } catch (error: unknown) { diff --git a/npm-packages/convex/src/cli/lib/indexes.ts b/npm-packages/convex/src/cli/lib/indexes.ts index 2cef9fb6..92a3dcd4 100644 --- a/npm-packages/convex/src/cli/lib/indexes.ts +++ b/npm-packages/convex/src/cli/lib/indexes.ts @@ -1,7 +1,6 @@ import chalk from "chalk"; import path from "path"; import { bundleSchema } from "../../bundler/index.js"; -import { version } from "../version.js"; import { Context, changeSpinner, @@ -63,14 +62,10 @@ export async function pushSchema( changeSpinner(ctx, "Checking for index or schema changes..."); let data: PrepareSchemaResponse; - const fetch = deploymentFetch(origin); + const fetch = deploymentFetch(origin, adminKey); try { const res = await fetch("/api/prepare_schema", { method: "POST", - headers: { - "Convex-Client": `npm-cli-${version}`, - "Content-Type": "application/json", - }, body: JSON.stringify({ bundle: bundles[0], adminKey, @@ -99,16 +94,10 @@ async function waitForReadySchema( schemaId: string, ): Promise { const path = `api/schema_state/${schemaId}`; - const depFetch = deploymentFetch(origin); + const depFetch = deploymentFetch(origin, adminKey); const fetch = async () => { try { - const resp = await depFetch(path, { - headers: { - Authorization: `Convex ${adminKey}`, - "Convex-Client": `npm-cli-${version}`, - "Content-Type": "application/json", - }, - }); + const resp = await depFetch(path, { method: "GET" }); const data: SchemaStateResponse = await resp.json(); return data; } catch (err: unknown) { diff --git a/npm-packages/convex/src/cli/lib/logs.ts b/npm-packages/convex/src/cli/lib/logs.ts index 88059a8d..7f0bb41a 100644 --- a/npm-packages/convex/src/cli/lib/logs.ts +++ b/npm-packages/convex/src/cli/lib/logs.ts @@ -4,7 +4,6 @@ import { logOutput, logWarning, } from "../../bundler/context.js"; -import { version } from "../version.js"; import { nextBackoff } from "../dev.js"; import chalk from "chalk"; import { deploymentFetch } from "./utils.js"; @@ -23,18 +22,13 @@ export async function watchLogs( history?: number | boolean; }, ) { - const authHeader = createAuthHeader(adminKey); let numFailures = 0; let isFirst = true; let cursorMs = 0; for (;;) { try { - const { entries, newCursor } = await pollUdfLog( - cursorMs, - url, - authHeader, - ); + const { entries, newCursor } = await pollUdfLog(cursorMs, url, adminKey); cursorMs = newCursor; numFailures = 0; // The first execution, we just want to fetch the current head cursor so we don't send stale @@ -75,10 +69,6 @@ export async function watchLogs( } } -function createAuthHeader(adminKey: string): string { - return `Convex ${adminKey}`; -} - type UdfType = "Query" | "Mutation" | "Action" | "HttpAction"; type StructuredLogLine = { @@ -104,14 +94,11 @@ type UdfExecutionResponse = { async function pollUdfLog( cursor: number, url: string, - authHeader: string, + adminKey: string, ): Promise<{ entries: UdfExecutionResponse[]; newCursor: number }> { - const fetch = deploymentFetch(url); + const fetch = deploymentFetch(url, adminKey); const response = await fetch(`/api/stream_function_logs?cursor=${cursor}`, { - headers: { - Authorization: authHeader, - "Convex-Client": `npm-cli-${version}`, - }, + method: "GET", }); return await response.json(); } diff --git a/npm-packages/convex/src/cli/lib/utils.ts b/npm-packages/convex/src/cli/lib/utils.ts index 380555f9..15c6509b 100644 --- a/npm-packages/convex/src/cli/lib/utils.ts +++ b/npm-packages/convex/src/cli/lib/utils.ts @@ -913,7 +913,65 @@ function retryDelay( return delay + randomSum; } -export function deploymentFetch( +function deploymentFetchRetryOn(onError?: (err: any) => void, method?: string) { + return function ( + _attempt: number, + error: Error | null, + response: Response | null, + ) { + if (onError && error !== null) { + onError(error); + } + + // Retry on network errors. + if (error) { + // TODO filter out all SSL errors + // https://github.com/nodejs/node/blob/8a41d9b636be86350cd32847c3f89d327c4f6ff7/src/crypto/crypto_common.cc#L218-L245 + return true; + } + // Retry on 404s since these can sometimes happen with newly created + // deployments for POSTs. + if (response?.status === 404) { + return true; + } + + // Whatever the error code it doesn't hurt to retry idempotent requests. + if ( + response && + !response.ok && + method && + IDEMPOTENT_METHODS.includes(method.toUpperCase()) + ) { + // ...but it's a bit annoying to wait for things we know won't succced + if ( + [ + 400, // Bad Request + 401, // Unauthorized + 402, // PaymentRequired + 403, // Forbidden + 405, // Method Not Allowed + 406, // Not Acceptable + 412, // Precondition Failed + 413, // Payload Too Large + 414, // URI Too Long + 415, // Unsupported Media Type + 416, // Range Not Satisfiable + ].includes(response.status) + ) { + return false; + } + return true; + } + + return false; + }; +} + +/** + * Unlike `deploymentFetch`, this does not add on any headers, so the caller + * must supply any headers. + */ +export function bareDeploymentFetch( deploymentUrl: string, onError?: (err: any) => void, ): typeof throwingFetch { @@ -927,59 +985,48 @@ export function deploymentFetch( const func = throwingFetch(url, { retries: 6, retryDelay, - retryOn: function ( - _attempt: number, - error: Error | null, - response: Response | null, - ) { - if (onError && error !== null) { - onError(error); - } - - // Retry on network errors. - if (error) { - // TODO filter out all SSL errors - // https://github.com/nodejs/node/blob/8a41d9b636be86350cd32847c3f89d327c4f6ff7/src/crypto/crypto_common.cc#L218-L245 - return true; - } - // Retry on 404s since these can sometimes happen with newly created - // deployments for POSTs. - if (response?.status === 404) { - return true; - } + retryOn: deploymentFetchRetryOn(onError, options?.method), + ...options, + }); + return func; + }; +} - const method = options?.method?.toUpperCase(); - // Whatever the error code it doesn't hurt to retry idempotent requests. - if ( - response && - !response.ok && - method && - IDEMPOTENT_METHODS.includes(method) - ) { - // ...but it's a bit annoying to wait for things we know won't succced - if ( - [ - 400, // Bad Request - 401, // Unauthorized - 402, // PaymentRequired - 403, // Forbidden - 405, // Method Not Allowed - 406, // Not Acceptable - 412, // Precondition Failed - 413, // Payload Too Large - 414, // URI Too Long - 415, // Unsupported Media Type - 416, // Range Not Satisfiable - ].includes(response.status) - ) { - return false; - } - return true; - } +/** + * This returns a `fetch` function that will fetch against `deploymentUrl`. + * + * It will also set the `Authorization` header, `Content-Type` header, and + * the `Convex-Client` header if they are not set in the `fetch`. + */ +export function deploymentFetch( + deploymentUrl: string, + adminKey: string, + onError?: (err: any) => void, +): typeof throwingFetch { + return (resource: RequestInfo | URL, options: RequestInit | undefined) => { + const url = + resource instanceof URL + ? resource.pathname + : typeof resource === "string" + ? new URL(resource, deploymentUrl) + : new URL(resource.url, deploymentUrl); - return false; - }, + const headers = new Headers(options?.headers || {}); + if (!headers.has("Authorization")) { + headers.set("Authorization", `Convex ${adminKey}`); + } + if (!headers.has("Content-Type")) { + headers.set("Content-Type", "application/json"); + } + if (!headers.has("Convex-Client")) { + headers.set("Convex-Client", `npm-cli-${version}`); + } + const func = throwingFetch(url, { + retries: 6, + retryDelay, + retryOn: deploymentFetchRetryOn(onError, options?.method), ...options, + headers, }); return func; }; diff --git a/npm-packages/convex/src/cli/network_test.ts b/npm-packages/convex/src/cli/network_test.ts index 33de041b..16063942 100644 --- a/npm-packages/convex/src/cli/network_test.ts +++ b/npm-packages/convex/src/cli/network_test.ts @@ -15,7 +15,11 @@ import { import * as net from "net"; import * as dns from "dns"; import * as crypto from "crypto"; -import { deploymentFetch, formatDuration, formatSize } from "./lib/utils.js"; +import { + bareDeploymentFetch, + formatDuration, + formatSize, +} from "./lib/utils.js"; import chalk from "chalk"; const ipFamilyNumbers = { ipv4: 4, ipv6: 6, auto: 0 } as const; @@ -232,7 +236,7 @@ async function checkHttpOnce( const start = performance.now(); // Be sure to use the same `deploymentFetch` we use elsewhere so we're actually // getting coverage of our network stack. - const fetch = deploymentFetch(url); + const fetch = bareDeploymentFetch(url); const instanceNameUrl = new URL("/instance_name", url); // Set `maxRedirects` to 0 so our HTTP test doesn't try HTTPS. const resp = await fetch(instanceNameUrl.toString(), { @@ -256,7 +260,7 @@ async function checkHttpOnce( async function checkEcho(ctx: Context, url: string, size: number) { try { const start = performance.now(); - const fetch = deploymentFetch(url, (err) => { + const fetch = bareDeploymentFetch(url, (err) => { logFailure( ctx, chalk.red(`FAIL: echo ${formatSize(size)} (${err}), retrying...`),