polish: improve consistency of warnings and errors (#848)

* polish: improve consistency of warnings and errors

Related to #377

.changeset/dirty-knives-exercise.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+polish: improve consistency of warnings and errors
+
+Related to #377

.vscode/settings.json

@@ -4,6 +4,7 @@
     "cfetch",
     "clipboardy",
     "cloudflared",
+    "Codespaces",
     "esbuild",
     "eslintcache",
     "execa",

packages/wrangler/src/__tests__/configuration.test.ts

@@ -323,7 +323,7 @@ describe("normalizeAndValidateConfig()", () => {
         `);
         expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
-            - 🚨 NO LONGER SUPPORTED: \\"site.entry-point\\":
+            - NO LONGER SUPPORTED: \\"site.entry-point\\":
               The \`site.entry-point\` config field is no longer used.
               The entry-point should be specified via the command line or the \`main\` config field."
         `);
@@ -534,9 +534,9 @@ describe("normalizeAndValidateConfig()", () => {
         expect(diagnostics.hasWarnings()).toBe(true);
         expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
-            - ⚠️  DEPRECATION: \\"type\\":
+            - DEPRECATION: \\"type\\":
               DO NOT USE THIS. Most common features now work out of the box with wrangler, including modules, jsx, typescript, etc. If you need anything more, use a custom build.
-            - ⚠️  DEPRECATION: \\"webpack_config\\":
+            - DEPRECATION: \\"webpack_config\\":
               DO NOT USE THIS. Most common features now work out of the box with wrangler, including modules, jsx, typescript, etc. If you need anything more, use a custom build."
         `);
       });
@@ -832,15 +832,15 @@ describe("normalizeAndValidateConfig()", () => {
       expect(normalizePath(diagnostics.renderWarnings()))
         .toMatchInlineSnapshot(`
         "Processing project/wrangler.toml configuration:
-          - ⚠️  DEPRECATION: \\"build.upload.format\\":
+          - DEPRECATION: \\"build.upload.format\\":
             The format is inferred automatically from the code.
-          - ⚠️  DEPRECATION: \\"build.upload.main\\":
+          - DEPRECATION: \\"build.upload.main\\":
             Delete the \`build.upload.main\` and \`build.upload.dir\` fields.
             Then add the top level \`main\` field to your configuration file:
             \`\`\`
             main = \\"src/index.ts\\"
             \`\`\`
-          - ⚠️  DEPRECATION: \\"build.upload.dir\\":
+          - DEPRECATION: \\"build.upload.dir\\":
             Use the top level \\"main\\" field or a command-line argument to specify the entry-point for the Worker.
           - DEPRECATION: The \`build.upload.rules\` config field is no longer used, the rules should be specified via the \`rules\` config field. Delete the \`build.upload\` field from the configuration file, and add this:
             \`\`\`
@@ -1582,9 +1582,9 @@ describe("normalizeAndValidateConfig()", () => {
         expect(diagnostics.hasWarnings()).toBe(true);
         expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
           "Processing wrangler configuration:
-            - ⚠️  DEPRECATION: \\"zone_id\\":
+            - DEPRECATION: \\"zone_id\\":
               This is unnecessary since we can deduce this from routes directly.
-            - ⚠️  DEPRECATION: \\"experimental_services\\":
+            - DEPRECATION: \\"experimental_services\\":
               The \\"experimental_services\\" field is no longer supported. Instead, use [[unsafe.bindings]] to enable experimental features. Add this to your wrangler.toml:
               \`\`\`
               [[unsafe.bindings]]
@@ -2806,9 +2806,9 @@ describe("normalizeAndValidateConfig()", () => {
           "Processing wrangler configuration:
 
             - \\"env.ENV1\\" environment configuration
-              - ⚠️  DEPRECATION: \\"zone_id\\":
+              - DEPRECATION: \\"zone_id\\":
                 This is unnecessary since we can deduce this from routes directly.
-              - ⚠️  DEPRECATION: \\"experimental_services\\":
+              - DEPRECATION: \\"experimental_services\\":
                 The \\"experimental_services\\" field is no longer supported. Instead, use [[unsafe.bindings]] to enable experimental features. Add this to your wrangler.toml:
                 \`\`\`
                 [[unsafe.bindings]]

packages/wrangler/src/__tests__/dev.test.tsx

@@ -40,7 +40,7 @@ describe("wrangler dev", () => {
       expect(std.out).toMatchInlineSnapshot(`""`);
       expect(std.warn.replaceAll(currentDate, "<current-date>"))
         .toMatchInlineSnapshot(`
-        "No compatibility_date was specified. Using today's date: <current-date>.
+        "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mNo compatibility_date was specified. Using today's date: <current-date>.
         Add one to your wrangler.toml file:
         \`\`\`
         compatibility_date = \\"<current-date>\\"
@@ -49,7 +49,9 @@ describe("wrangler dev", () => {
         \`\`\`
         --compatibility-date=<current-date>
         \`\`\`
-        See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
+        See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information.
+
+        "
       `);
       expect(std.err).toMatchInlineSnapshot(`""`);
     });
@@ -77,11 +79,14 @@ describe("wrangler dev", () => {
         `"Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`main\` config field."`
       );
 
-      expect(std.out).toMatchInlineSnapshot(`""`);
+      expect(std.out).toMatchInlineSnapshot(`
+        "
+        If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
+      `);
       expect(std.err).toMatchInlineSnapshot(`
-        "Missing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`main\` config field.
+        "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mMissing entry-point: The entry-point should be specified via the command line (e.g. \`wrangler dev path/to/script\`) or the \`main\` config field.[0m
 
-        [32m%s[0m If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
+        "
       `);
     });
 
@@ -389,7 +394,7 @@ describe("wrangler dev", () => {
         watch_dir: "src",
       });
       expect(std.out).toMatchInlineSnapshot(
-        `"running: node -e \\"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\\""`
+        `"Running custom build: node -e \\"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\\""`
       );
       expect(std.err).toMatchInlineSnapshot(`""`);
       expect(std.warn).toMatchInlineSnapshot(`""`);
@@ -411,7 +416,7 @@ describe("wrangler dev", () => {
         `);
 
         expect(std.out).toMatchInlineSnapshot(
-          `"running: echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123) } }\\" > index.js"`
+          `"Running custom build: echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123) } }\\" > index.js"`
         );
         expect(std.err).toMatchInlineSnapshot(`""`);
         expect(std.warn).toMatchInlineSnapshot(`""`);
@@ -431,13 +436,15 @@ describe("wrangler dev", () => {
       ).rejects.toThrowErrorMatchingInlineSnapshot(
         `"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\""`
       );
-      expect(std.out).toMatchInlineSnapshot(
-        `"running: node -e \\"console.log('custom build');\\""`
-      );
+      expect(std.out).toMatchInlineSnapshot(`
+        "Running custom build: node -e \\"console.log('custom build');\\"
+
+        If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
+      `);
       expect(std.err).toMatchInlineSnapshot(`
-        "Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\"
+        "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mCould not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\"[0m
 
-        [32m%s[0m If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
+        "
       `);
       expect(std.warn).toMatchInlineSnapshot(`""`);
     });
@@ -469,9 +476,11 @@ describe("wrangler dev", () => {
       );
       expect(std.out).toMatchInlineSnapshot(`""`);
       expect(std.warn).toMatchInlineSnapshot(`
-        "Setting upstream-protocol to http is not currently implemented.
+        "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mSetting upstream-protocol to http is not currently implemented.
         If this is required in your project, please add your use case to the following issue:
-        https://github.com/cloudflare/wrangler2/issues/583."
+        https://github.com/cloudflare/wrangler2/issues/583.
+
+        "
       `);
       expect(std.err).toMatchInlineSnapshot(`""`);
     });
@@ -595,11 +604,13 @@ describe("wrangler dev", () => {
       expect((Dev as jest.Mock).mock.calls[0][0].ip).toEqual("localhost");
       expect(std.out).toMatchInlineSnapshot(`""`);
       expect(std.warn).toMatchInlineSnapshot(`
-        "WARNING: You have Durable Object bindings, which are not defined locally in the worker being developed.
+        "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mWARNING: You have Durable Object bindings that are not defined locally in the worker being developed.
         Be aware that changes to the data stored in these Durable Objects will be permanent and affect the live instances.
         Remote Durable Objects that are affected:
         - {\\"name\\":\\"NAME_2\\",\\"class_name\\":\\"CLASS_2\\",\\"script_name\\":\\"SCRIPT_A\\"}
-        - {\\"name\\":\\"NAME_4\\",\\"class_name\\":\\"CLASS_4\\",\\"script_name\\":\\"SCRIPT_B\\"}"
+        - {\\"name\\":\\"NAME_4\\",\\"class_name\\":\\"CLASS_4\\",\\"script_name\\":\\"SCRIPT_B\\"}
+
+        "
       `);
       expect(std.err).toMatchInlineSnapshot(`""`);
     });

packages/wrangler/src/__tests__/guess-worker-format.test.ts

@@ -76,8 +76,10 @@ describe("guess worker format", () => {
       undefined
     );
     expect(guess).toBe("service-worker");
-    expect(std.warn).toMatchInlineSnapshot(
-      `"The entrypoint index.ts has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using \\"service-worker\\" format..."`
-    );
+    expect(std.warn).toMatchInlineSnapshot(`
+      "▲ [WARNING] The entrypoint index.ts has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using \\"service-worker\\" format...
+
+      "
+    `);
   });
 });

packages/wrangler/src/__tests__/helpers/mock-console.ts

@@ -5,11 +5,15 @@ import * as util from "node:util";
  * assert on the values they're called with in our tests.
  */
 
-let logSpy: jest.SpyInstance,
+let debugSpy: jest.SpyInstance,
+  logSpy: jest.SpyInstance,
   errorSpy: jest.SpyInstance,
   warnSpy: jest.SpyInstance;
 
 const std = {
+  get debug() {
+    return normalizeOutput(debugSpy);
+  },
   get out() {
     return normalizeOutput(logSpy);
   },
@@ -22,8 +26,8 @@ const std = {
 };
 
 function normalizeOutput(spy: jest.SpyInstance): string {
-  return stripTrailingWhitespace(
-    normalizeSlashes(stripTimings(captureCalls(spy)))
+  return normalizeErrorMarkers(
+    stripTrailingWhitespace(normalizeSlashes(stripTimings(captureCalls(spy))))
   );
 }
 
@@ -35,18 +39,29 @@ function captureCalls(spy: jest.SpyInstance): string {
 
 export function mockConsoleMethods() {
   beforeEach(() => {
+    debugSpy = jest.spyOn(console, "debug").mockImplementation();
     logSpy = jest.spyOn(console, "log").mockImplementation();
     errorSpy = jest.spyOn(console, "error").mockImplementation();
     warnSpy = jest.spyOn(console, "warn").mockImplementation();
   });
   afterEach(() => {
+    debugSpy.mockRestore();
     logSpy.mockRestore();
     errorSpy.mockRestore();
     warnSpy.mockRestore();
   });
   return std;
 }
 
+/**
+ * Normalize error `X` markers.
+ *
+ * Windows gets a different character.
+ */
+function normalizeErrorMarkers(str: string): string {
+  return str.replaceAll("✘", "X");
+}
+
 /**
  * Ensure slashes in the `str` are OS file-system agnostic.
  *

packages/wrangler/src/__tests__/https-options.test.ts

@@ -97,8 +97,10 @@ describe("getHttpsOptions()", () => {
       `"Generating new self-signed certificate..."`
     );
     expect(std.warn).toMatchInlineSnapshot(`
-      "Unable to cache generated self-signed certificate in home/.wrangler/local-cert.
-      ERROR: Cannot write file"
+      "▲ [WARNING] Unable to cache generated self-signed certificate in home/.wrangler/local-cert.
+      ERROR: Cannot write file
+
+      "
     `);
     expect(std.err).toMatchInlineSnapshot(`""`);
   });

packages/wrangler/src/__tests__/index.test.ts

@@ -92,8 +92,9 @@ describe("wrangler", () => {
           -h, --help        Show help  [boolean]
           -v, --version     Show version number  [boolean]
               --legacy-env  Use legacy environments  [boolean]
+        X [ERROR] Unknown argument: invalid-command
 
-        Unknown argument: invalid-command"
+        "
       `);
     });
   });
@@ -845,22 +846,22 @@ describe("wrangler", () => {
     it("should throw an error if the deprecated command is used with positional arguments", async () => {
       await expect(runWrangler("preview GET")).rejects
         .toThrowErrorMatchingInlineSnapshot(`
-              "⚠️  DEPRECATION:
+              "DEPRECATION:
               The \`wrangler preview\` command has been deprecated.
               Try using \`wrangler dev\` to to try out a worker during development.
               "
             `);
       await expect(runWrangler(`preview GET "SomeBody"`)).rejects
         .toThrowErrorMatchingInlineSnapshot(`
-              "⚠️  DEPRECATION:
+              "DEPRECATION:
               The \`wrangler preview\` command has been deprecated.
               Try using \`wrangler dev\` to to try out a worker during development.
               "
             `);
     });
   });
 
-  describe("subcommand implicit help ran on imcomplete command execution", () => {
+  describe("subcommand implicit help ran on incomplete command execution", () => {
     function endEventLoop() {
       return new Promise((resolve) => setImmediate(resolve));
     }
@@ -970,15 +971,15 @@ describe("wrangler", () => {
     it("should print a deprecation message for 'generate'", async () => {
       await runWrangler("generate").catch((err) => {
         expect(err.message).toMatchInlineSnapshot(`
-          "⚠️  DEPRECATION:
+          "DEPRECATION:
           \`wrangler generate\` has been deprecated, please refer to https://github.com/cloudflare/wrangler2/blob/main/docs/deprecations.md#generate for alternatives"
         `);
       });
     });
     it("should print a deprecation message for 'build'", async () => {
       await runWrangler("build").catch((err) => {
         expect(err.message).toMatchInlineSnapshot(`
-          "⚠️  DEPRECATION:
+          "DEPRECATION:
           \`wrangler build\` has been deprecated, please refer to https://github.com/cloudflare/wrangler2/blob/main/docs/deprecations.md#build for alternatives"
         `);
       });