Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup dev registry messaging #7172

Merged
merged 5 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/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(`
"▲ [WARNING] This is a warnOnce message
"▲ [WARNING] This is a once.warn message

"
`);
});

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.");
CarmenPopoviciu marked this conversation as resolved.
Show resolved Hide resolved

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
Loading