Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/pink-beees-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Remove dummy auth from SDK setup
5 changes: 5 additions & 0 deletions .changeset/pink-bees-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Add `WRANGLER_TRACE_ID` environment variable to support internal testing
2 changes: 1 addition & 1 deletion packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
"chalk": "^5.2.0",
"chokidar": "^4.0.1",
"cli-table3": "^0.6.3",
"cloudflare": "^4.5.0",
"cloudflare": "^5.1.0",
"cmd-shim": "^4.1.0",
"command-exists": "^1.2.9",
"concurrently": "^8.2.2",
Expand Down
16 changes: 11 additions & 5 deletions packages/wrangler/src/cfetch/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import assert from "node:assert";
import Cloudflare from "cloudflare";
import { fetch, FormData, Headers, Request, Response } from "undici";
import { version as wranglerVersion } from "../../package.json";
import { getCloudflareApiBaseUrl } from "../environment-variables/misc-variables";
import {
getCloudflareApiBaseUrl,
getTraceHeader,
} from "../environment-variables/misc-variables";
import { UserError } from "../errors";
import { logger } from "../logger";
import { APIError, parseJSON } from "../parse";
Expand Down Expand Up @@ -73,10 +76,6 @@ export function createCloudflareClient(complianceConfig: ComplianceConfig) {
await logResponse(response);
return response;
},
// We inject authentication using Wrangler's existing auth setup and a custom fetcher (see above for the custom fetch)
// However, `cloudflare` doesn't like not being given an API token, so we provide a dummy one
// Otherwise, errors like "Could not resolve authentication method." are thrown.
apiToken: "dummy",
baseURL: getCloudflareApiBaseUrl(complianceConfig),
});
}
Expand Down Expand Up @@ -104,6 +103,7 @@ export async function performApiFetch(
const headers = cloneHeaders(new Headers(init.headers));
addAuthorizationHeader(headers, apiToken);
addUserAgent(headers);
addTraceHeader(headers);

const queryString = queryParams ? `?${queryParams.toString()}` : "";
logger.debug(
Expand Down Expand Up @@ -243,6 +243,12 @@ export function addUserAgent(headers: Headers): void {
headers.set("User-Agent", `wrangler/${wranglerVersion}`);
}

export function addTraceHeader(headers: Headers): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe update the name to indicate that it might not add anything?
i.e. addTraceHeaderWhenNeeded or maybeAddTraceHeader

if (getTraceHeader()) {
headers.set("Cf-Trace-Id", getTraceHeader() as string);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: avoid the cast and calling the fn twice:

Suggested change
if (getTraceHeader()) {
headers.set("Cf-Trace-Id", getTraceHeader() as string);
}
const traceHeader = getTraceHeader();
if (traceHeader) {
headers.set("Cf-Trace-Id", traceHeader);
}

}

/**
* The implementation for fetching a kv value from the cloudflare API.
* We special-case this one call, because it's the only API call that
Expand Down
3 changes: 2 additions & 1 deletion packages/wrangler/src/environment-variables/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ type VariableNames =
/** Docker host configuration (handled separately from environment variable factory). */
| "WRANGLER_DOCKER_HOST"
/** Docker host configuration (handled separately from environment variable factory). */
| "DOCKER_HOST";
| "DOCKER_HOST"
| "WRANGLER_TRACE_ID";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make it clearer that this is not in the // ## Docker Configuration section and add a comment.

Is this wrangler specific or should rather be CLOUDFLARE_...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would // ## Development & Local Testing be a better section to add that to?


type DeprecatedNames =
| "CF_ACCOUNT_ID"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,3 +290,7 @@ export const getCloudflareIncludeProcessEnvFromEnv =
variableName: "CLOUDFLARE_INCLUDE_PROCESS_ENV",
defaultValue: false,
});

export const getTraceHeader = getEnvironmentVariableFactory({
variableName: "WRANGLER_TRACE_ID",
});
10 changes: 5 additions & 5 deletions pnpm-lock.yaml

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

Loading