Skip to content

Commit

Permalink
Cleanup dev registry messaging (#7172)
Browse files Browse the repository at this point in the history
  • Loading branch information
penalosa authored Nov 7, 2024
1 parent f292294 commit 3dce388
Show file tree
Hide file tree
Showing 13 changed files with 97 additions and 55 deletions.
5 changes: 5 additions & 0 deletions .changeset/large-otters-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Clarify dev registry messaging around locally connected services. The connection status of local service bindings & durable object bindings is now indicated by `connected` or `not connected` next to their entry in the bindings summary. For more details, refer to https://developers.cloudflare.com/workers/runtime-apis/bindings/service-bindings/#local-development
27 changes: 14 additions & 13 deletions packages/wrangler/e2e/__snapshots__/pages-dev.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ exports[`Pages 'wrangler pages dev --no-x-dev-env' > should merge (with override
▲ [WARNING] This worker is bound to live services: SERVICE_BINDING_1_TOML (SERVICE_NAME_1_TOML), SERVICE_BINDING_2_TOML (SERVICE_NAME_2_TOML)
Your worker has access to the following bindings:
- Durable Objects:
- DO_BINDING_1_TOML: NEW_DO_1 (defined in 🔴 NEW_DO_SCRIPT_1)
- DO_BINDING_2_TOML: DO_2_TOML (defined in 🔴 DO_SCRIPT_2_TOML)
- DO_BINDING_3_ARGS: DO_3_ARGS (defined in 🔴 DO_SCRIPT_3_ARGS)
- DO_BINDING_1_TOML: NEW_DO_1 (defined in NEW_DO_SCRIPT_1 [not connected])
- DO_BINDING_2_TOML: DO_2_TOML (defined in DO_SCRIPT_2_TOML [not connected])
- DO_BINDING_3_ARGS: DO_3_ARGS (defined in DO_SCRIPT_3_ARGS [not connected])
- KV Namespaces:
- KV_BINDING_1_TOML: NEW_KV_ID_1 (local)
- KV_BINDING_2_TOML: KV_ID_2_TOML (local)
Expand All @@ -26,15 +26,16 @@ Your worker has access to the following bindings:
- R2_BINDING_2_TOML: R2_BUCKET_2_TOML (local)
- R2_BINDING_3_TOML: R2_BUCKET_3_ARGS (local)
- Services:
- SERVICE_BINDING_1_TOML: 🔴 NEW_SERVICE_NAME_1
- SERVICE_BINDING_2_TOML: 🔴 SERVICE_NAME_2_TOML
- SERVICE_BINDING_3_TOML: 🔴 SERVICE_NAME_3_ARGS
- SERVICE_BINDING_1_TOML: NEW_SERVICE_NAME_1 [not connected]
- SERVICE_BINDING_2_TOML: SERVICE_NAME_2_TOML [not connected]
- SERVICE_BINDING_3_TOML: SERVICE_NAME_3_ARGS [not connected]
- AI:
- Name: AI_BINDING_2_TOML
- Vars:
- VAR1: "(hidden)"
- VAR2: "VAR_2_TOML"
- VAR3: "(hidden)"
Service bindings & durable object bindings connect to other \`wrangler dev\` processes running locally, with their connection status indicated by [connected] or [not connected]. For more details, refer to https://developers.cloudflare.com/workers/runtime-apis/bindings/service-bindings/#local-development
▲ [WARNING] ⎔ Support for service bindings in local mode is experimental and may change.
▲ [WARNING] ⎔ Support for external Durable Objects in local mode is experimental and may change.
"
Expand All @@ -49,9 +50,9 @@ exports[`Pages 'wrangler pages dev --x-dev-env' > should merge (with override) \
- {"name":"DO_BINDING_2_TOML","class_name":"DO_2_TOML","script_name":"DO_SCRIPT_2_TOML"}
Your worker has access to the following bindings:
- Durable Objects:
- DO_BINDING_1_TOML: NEW_DO_1 (defined in 🔴 NEW_DO_SCRIPT_1)
- DO_BINDING_2_TOML: DO_2_TOML (defined in 🔴 DO_SCRIPT_2_TOML)
- DO_BINDING_3_ARGS: DO_3_ARGS (defined in 🔴 DO_SCRIPT_3_ARGS)
- DO_BINDING_1_TOML: NEW_DO_1 (defined in NEW_DO_SCRIPT_1 [not connected])
- DO_BINDING_2_TOML: DO_2_TOML (defined in DO_SCRIPT_2_TOML [not connected])
- DO_BINDING_3_ARGS: DO_3_ARGS (defined in DO_SCRIPT_3_ARGS [not connected])
- KV Namespaces:
- KV_BINDING_1_TOML: NEW_KV_ID_1 (local)
- KV_BINDING_2_TOML: KV_ID_2_TOML (local)
Expand All @@ -65,16 +66,16 @@ Your worker has access to the following bindings:
- R2_BINDING_2_TOML: R2_BUCKET_2_TOML (local)
- R2_BINDING_3_TOML: R2_BUCKET_3_ARGS (local)
- Services:
- SERVICE_BINDING_1_TOML: 🔴 NEW_SERVICE_NAME_1
- SERVICE_BINDING_2_TOML: 🔴 SERVICE_NAME_2_TOML
- SERVICE_BINDING_3_TOML: 🔴 SERVICE_NAME_3_ARGS
- SERVICE_BINDING_1_TOML: NEW_SERVICE_NAME_1 [not connected]
- SERVICE_BINDING_2_TOML: SERVICE_NAME_2_TOML [not connected]
- SERVICE_BINDING_3_TOML: SERVICE_NAME_3_ARGS [not connected]
- AI:
- Name: AI_BINDING_2_TOML
- Vars:
- VAR1: "(hidden)"
- VAR2: "VAR_2_TOML"
- VAR3: "(hidden)"
▲ [WARNING] This worker is bound to live services: SERVICE_BINDING_1_TOML (NEW_SERVICE_NAME_1), SERVICE_BINDING_3_TOML (SERVICE_NAME_3_ARGS), SERVICE_BINDING_2_TOML (SERVICE_NAME_2_TOML)
Service bindings & durable object bindings connect to other \`wrangler dev\` processes running locally, with their connection status indicated by [connected] or [not connected]. For more details, refer to https://developers.cloudflare.com/workers/runtime-apis/bindings/service-bindings/#local-development
▲ [WARNING] Using Workers AI always accesses your Cloudflare account in order to run AI models, and so will incur usage charges even in local development.
"
`;
5 changes: 5 additions & 0 deletions packages/wrangler/e2e/dev-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
import { WranglerE2ETestHelper } from "./helpers/e2e-wrangler-test";
import { fetchText } from "./helpers/fetch-text";
import { generateResourceName } from "./helpers/generate-resource-name";
import { normalizeOutput } from "./helpers/normalize";
import { seed as baseSeed, makeRoot } from "./helpers/setup";
import type { RequestInit } from "undici";

Expand Down Expand Up @@ -180,6 +181,10 @@ describe.each([
async () => await expect(fetchText(url)).resolves.toBe("hello world"),
{ interval: 1000, timeout: 10_000 }
);

expect(normalizeOutput(workerA.currentOutput)).toContain(
"bindings connect to other `wrangler dev` processes running locally"
);
});

it("can fetch b through a (start a, start b)", async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/e2e/pages-dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,15 @@ describe.each([
expect(normalizeOutput(worker.currentOutput)).toContain(
dedent`Your worker has access to the following bindings:
- Durable Objects:
- TEST_DO: TestDurableObject (defined in 🔴 a)
- TEST_DO: TestDurableObject (defined in a [not connected])
- KV Namespaces:
- TEST_KV: TEST_KV (local)
- D1 Databases:
- TEST_D1: local-TEST_D1 (TEST_D1) (local)
- R2 Buckets:
- TEST_R2: TEST_R2 (local)
- Services:
- TEST_SERVICE: 🔴 test-worker
- TEST_SERVICE: test-worker [not connected]
`
);
});
Expand Down
24 changes: 8 additions & 16 deletions packages/wrangler/src/__tests__/dev.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1229,9 +1229,9 @@ describe.sequential("wrangler dev", () => {
"Your worker has access to the following bindings:
- Durable Objects:
- NAME_1: CLASS_1
- NAME_2: CLASS_2 (defined in 🔴 SCRIPT_A)
- NAME_2: CLASS_2 (defined in SCRIPT_A [not connected])
- NAME_3: CLASS_3
- NAME_4: CLASS_4 (defined in 🔴 SCRIPT_B)
- NAME_4: CLASS_4 (defined in SCRIPT_B [not connected])
"
`);
expect(std.warn).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -1809,15 +1809,11 @@ describe.sequential("wrangler dev", () => {
expect(std.out).toMatchInlineSnapshot(`
"Your worker has access to the following bindings:
- Services:
- WorkerA: 🔴 A
- WorkerB: 🔴 B
- WorkerA: A [not connected]
- WorkerB: B [not connected]
"
`);
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] This worker is bound to live services: WorkerA (A), WorkerB (B@staging)
"
`);
expect(std.warn).toMatchInlineSnapshot(`""`);
});
});

Expand All @@ -1834,15 +1830,11 @@ describe.sequential("wrangler dev", () => {
expect(std.out).toMatchInlineSnapshot(`
"Your worker has access to the following bindings:
- Services:
- WorkerA: 🔴 A
- WorkerB: 🔴 B
"
`);
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] This worker is bound to live services: WorkerA (A), WorkerB (B@staging)
- WorkerA: A [not connected]
- WorkerB: B [not connected]
"
`);
expect(std.warn).toMatchInlineSnapshot(`""`);
});

it("should mask vars that were overriden in .dev.vars", async () => {
Expand Down
32 changes: 25 additions & 7 deletions packages/wrangler/src/__tests__/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,20 +204,38 @@ describe("logger", () => {
});
});

describe("warnOnce", () => {
describe("once", () => {
it("should only log the same message once", () => {
const logger = new Logger();
logger.warnOnce("This is a warnOnce message");
logger.warnOnce("This is a warnOnce message");
logger.warnOnce("This is a warnOnce message");
logger.warnOnce("This is a warnOnce message");
logger.warnOnce("This is a warnOnce message");
logger.once.warn("This is a once.warn message");
logger.once.warn("This is a once.warn message");
logger.once.warn("This is a once.warn message");
logger.once.warn("This is a once.warn message");
logger.once.warn("This is a once.warn message");

expect(std.warn).toMatchInlineSnapshot(`
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mThis is a warnOnce message[0m
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mThis is a once.warn message[0m
"
`);
});

it("should log once per log level", () => {
const logger = new Logger();
logger.once.warn("This is a once message");
logger.once.info("This is a once message");
logger.once.warn("This is a once message");
logger.once.warn("This is a once message");
logger.once.info("This is a once message");
logger.once.info("This is a once message");
logger.once.warn("This is a once message");

expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] This is a once message
"
`);
expect(std.info).toMatchInlineSnapshot(`"This is a once message"`);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ async function resolveConfig(
validateAssetsArgsAndConfig(resolved);

const services = extractBindingsOfType("service", resolved.bindings);
if (services && services.length > 0) {
if (services && services.length > 0 && resolved.dev?.remote) {
logger.warn(
`This worker is bound to live services: ${services
.map(
Expand Down
19 changes: 15 additions & 4 deletions packages/wrangler/src/config/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from "node:fs";
import chalk from "chalk";
import dotenv from "dotenv";
import { findUpSync } from "find-up";
import { FatalError, UserError } from "../errors";
Expand Down Expand Up @@ -236,6 +237,7 @@ export function printBindings(
local?: boolean;
} = {}
) {
let hasConnectionStatus = false;
const truncate = (item: string | Record<string, unknown>) => {
const s = typeof item === "string" ? item : JSON.stringify(item);
const maxLength = 40;
Expand Down Expand Up @@ -297,15 +299,16 @@ export function printBindings(
if (context.local) {
const registryDefinition = context.registry?.[script_name];

hasConnectionStatus = true;
if (
registryDefinition &&
registryDefinition.durableObjects.some(
(d) => d.className === class_name
)
) {
value += ` (defined in 🟢 ${script_name})`;
value += ` (defined in ${script_name} ${chalk.green("[connected]")})`;
} else {
value += ` (defined in 🔴 ${script_name})`;
value += ` (defined in ${script_name} ${chalk.red("[not connected]")})`;
}
} else {
value += ` (defined in ${script_name})`;
Expand Down Expand Up @@ -463,14 +466,16 @@ export function printBindings(

if (context.local) {
const registryDefinition = context.registry?.[service];
hasConnectionStatus = true;

if (
registryDefinition &&
(!entrypoint ||
registryDefinition.entrypointAddresses?.[entrypoint])
) {
value = `🟢 ` + value;
value = value + " " + chalk.green("[connected]");
} else {
value = `🔴 ` + value;
value = value + " " + chalk.red("[not connected]");
}
}
return {
Expand Down Expand Up @@ -637,6 +642,12 @@ export function printBindings(
].join("\n");

logger.log(message);

if (hasConnectionStatus) {
logger.once.info(
`\nService bindings & durable object bindings connect to other \`wrangler dev\` processes running locally, with their connection status indicated by ${chalk.green("[connected]")} or ${chalk.red("[not connected]")}. For more details, refer to https://developers.cloudflare.com/workers/runtime-apis/bindings/service-bindings/#local-development\n`
);
}
}

export function withConfig<T>(
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/deploy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export async function deployHandler(args: DeployArgs) {
}

if (config.workflows?.length) {
logger.warnOnce("Workflows is currently in open beta.");
logger.once.warn("Workflows is currently in open beta.");
}

validateAssetsArgsAndConfig(args, config);
Expand Down
26 changes: 18 additions & 8 deletions packages/wrangler/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function getLoggerLevel(): LoggerLevel {
const expected = Object.keys(LOGGER_LEVELS)
.map((level) => `"${level}"`)
.join(" | ");
logger.warnOnce(
logger.once.warn(
`Unrecognised WRANGLER_LOG value ${JSON.stringify(
fromEnv
)}, expected ${expected}, defaulting to "log"...`
Expand Down Expand Up @@ -107,14 +107,24 @@ export class Logger {
Logger.#afterLogHook?.();
}

static warnOnceHistory = new Set();
warnOnce(message: string) {
// using this.constructor.warnOnceHistory, instead of hard-coding Logger.warnOnceHistory, allows for subclassing
const { warnOnceHistory } = this.constructor as typeof Logger;
static onceHistory = new Set();
get once() {
return {
info: (...args: unknown[]) => this.doLogOnce("info", args),
log: (...args: unknown[]) => this.doLogOnce("log", args),
warn: (...args: unknown[]) => this.doLogOnce("warn", args),
error: (...args: unknown[]) => this.doLogOnce("error", args),
};
}
doLogOnce(messageLevel: Exclude<LoggerLevel, "none">, args: unknown[]) {
// using this.constructor.onceHistory, instead of hard-coding Logger.onceHistory, allows for subclassing
const { onceHistory } = this.constructor as typeof Logger;

const cacheKey = `${messageLevel}: ${args.join(" ")}`;

if (!warnOnceHistory.has(message)) {
warnOnceHistory.add(message);
this.warn(message);
if (!onceHistory.has(cacheKey)) {
onceHistory.add(cacheKey);
this.doLog(messageLevel, args);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/triggers/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export default async function triggersDeploy(
}

if (config.workflows?.length) {
logger.warnOnce("Workflows is currently in open beta.");
logger.once.warn("Workflows is currently in open beta.");

for (const workflow of config.workflows) {
deployments.push(
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/versions/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export async function versionsDeployHandler(args: VersionsDeployArgs) {
}

if (config.workflows?.length) {
logger.warnOnce("Workflows is currently in open beta.");
logger.once.warn("Workflows is currently in open beta.");
}

const versionCache: VersionCache = new Map();
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/versions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export async function versionsUploadHandler(
}

if (config.workflows?.length) {
logger.warnOnce("Workflows is currently in open beta.");
logger.once.warn("Workflows is currently in open beta.");
}

validateAssetsArgsAndConfig(
Expand Down

0 comments on commit 3dce388

Please sign in to comment.