better messaging when deploying large files (#1267)

Co-authored-by: Mike Bostock <[email protected]>
Co-authored-by: Philippe Rivière <[email protected]>

Author: 3 people

src/bin/observable.ts

@@ -2,6 +2,7 @@
 import type {ParseArgsConfig} from "node:util";
 import {parseArgs} from "node:util";
 import * as clack from "@clack/prompts";
+import wrapAnsi from "wrap-ansi";
 import {readConfig} from "../config.js";
 import {CliError} from "../error.js";
 import {faint, link, red} from "../tty.js";
@@ -199,29 +200,37 @@ try {
     }
   }
 } catch (error: any) {
+  const wrapWidth = process.stdout.columns ?? 80;
+  const bugMessage = "If you think this is a bug, please file an issue at";
+  const bugUrl = "https://github.com/observablehq/framework/issues";
+  const clackBugMessage = () => {
+    // clack.outro doesn't handle multiple lines well, so do it manually
+    console.log(`${faint("│\n│")}  ${bugMessage}\n${faint("└")}  ${link(bugUrl)}\n`);
+  };
+  const consoleBugMessage = () => {
+    console.error(`${bugMessage}\n↳ ${link(bugUrl)}\n`);
+  };
+
   if (error instanceof CliError) {
     if (error.print) {
       if (command && CLACKIFIED_COMMANDS.includes(command)) {
-        clack.outro(red(`Error: ${error.message}`));
+        clack.log.error(wrapAnsi(red(`Error: ${error.message}`), wrapWidth));
+        clackBugMessage();
       } else {
         console.error(red(error.message));
+        consoleBugMessage();
       }
     }
     process.exit(error.exitCode);
   } else {
     if (command && CLACKIFIED_COMMANDS.includes(command)) {
-      clack.log.error(`${red("Error:")} ${error.message}`);
+      clack.log.error(wrapAnsi(`${red("Error:")} ${error.message}`, wrapWidth));
       if (values.debug) {
         clack.outro("The full error follows");
         throw error;
       } else {
         clack.log.info("To see the full stack trace, run with the --debug flag.");
-        // clack.outro doesn't handle multiple lines well, so do it manually
-        console.log(
-          `${faint("│\n│")}  If you think this is a bug, please file an issue at\n${faint("└")}  ${link(
-            "https://github.com/observablehq/framework/issues"
-          )}\n`
-        );
+        clackBugMessage();
       }
     } else {
       console.error(`\n${red("Unexpected error:")} ${error.message}`);
@@ -230,11 +239,7 @@ try {
         throw error;
       } else {
         console.error("\nTip: To see the full stack trace, run with the --debug flag.\n");
-        console.error(
-          `If you think this is a bug, please file an issue at\n↳ ${link(
-            "https://github.com/observablehq/framework/issues"
-          )}\n`
-        );
+        consoleBugMessage();
       }
     }
   }

src/format.ts

@@ -13,3 +13,15 @@ export function formatLocaleDate(date: Date, locale: Intl.LocalesArgument = "en-
 function pad(number: number, length: number): string {
   return String(number).padStart(length, "0");
 }
+
+export function formatByteSize(n: number): string {
+  const suffixes = ["bytes", "KB", "MB", "GB"];
+  for (const suffix of suffixes) {
+    if (n < 1300) {
+      if (suffix === "bytes") return `${n} ${suffix}`;
+      return `${n > 100 ? n.toFixed(0) : n.toFixed(1)} ${suffix}`;
+    }
+    n /= 1000;
+  }
+  return `${n.toFixed(1)} TB`;
+}

src/observableApiClient.ts

@@ -1,6 +1,7 @@
 import fs from "node:fs/promises";
 import type {ClackEffects} from "./clack.js";
 import {CliError, HttpError, isApiError} from "./error.js";
+import {formatByteSize} from "./format.js";
 import type {ApiKey} from "./observableApiConfig.js";
 import {faint, red} from "./tty.js";
 
@@ -87,9 +88,11 @@ export class ObservableApiClient {
       } catch (error) {
         // that's ok
       }
-      const error = new HttpError(`Unexpected response status ${JSON.stringify(response.status)}`, response.status, {
-        details
-      });
+      const error = new HttpError(
+        `Unexpected response status ${JSON.stringify(response.status)} for ${options.method ?? "GET"} ${url.href}`,
+        response.status,
+        {details}
+      );
 
       // check for version mismatch
       if (
@@ -183,7 +186,14 @@ export class ObservableApiClient {
     const blob = new Blob([contents]);
     body.append("file", blob);
     body.append("client_name", relativePath);
-    await this._fetch(url, {method: "POST", body});
+    try {
+      await this._fetch(url, {method: "POST", body});
+    } catch (error) {
+      const message = error instanceof Error ? error.message : `${error}`;
+      throw new CliError(`While uploading ${relativePath} (${formatByteSize(contents.length)}): ${message}`, {
+        cause: error
+      });
+    }
   }
 
   async postDeployUploaded(deployId: string): Promise<DeployInfo> {

test/deploy-test.ts

@@ -481,8 +481,9 @@ describe("deploy", () => {
       await deploy({...TEST_OPTIONS, maxConcurrency: 1}, effects);
       fail("Should have thrown an error");
     } catch (error) {
-      assert.ok(isHttpError(error));
-      assert.equal(error.statusCode, 500);
+      CliError.assert(error, {message: /While uploading index.html.*500/});
+      assert.ok(isHttpError(error.cause));
+      assert.equal(error.cause.statusCode, 500);
     }
 
     effects.close();

test/format-test.ts

@@ -1,5 +1,5 @@
 import assert from "node:assert";
-import {formatIsoDate, formatLocaleDate} from "../src/format.js";
+import {formatByteSize, formatIsoDate, formatLocaleDate} from "../src/format.js";
 
 describe("formatIsoDate", () => {
   it("returns an ISO 8601 string in local time", () => {
@@ -21,3 +21,22 @@ describe("formatLocaleDate", () => {
     assert.strictEqual(formatLocaleDate(new Date("2013-01-02T08:00:00Z"), "fr-FR"), "2 janv. 2013");
   });
 });
+
+describe("formatByteSize", () => {
+  it("returns a human-readable byte size", () => {
+    assert.strictEqual(formatByteSize(0), "0 bytes");
+    assert.strictEqual(formatByteSize(500), "500 bytes");
+    assert.strictEqual(formatByteSize(1200), "1200 bytes");
+    assert.strictEqual(formatByteSize(1_500), "1.5 KB");
+    assert.strictEqual(formatByteSize(3_000), "3.0 KB");
+    assert.strictEqual(formatByteSize(400_000), "400 KB");
+    assert.strictEqual(formatByteSize(50_000_000), "50.0 MB");
+    assert.strictEqual(formatByteSize(1_250_000_000), "1250 MB");
+    assert.strictEqual(formatByteSize(1_500_000_000), "1.5 GB");
+    assert.strictEqual(formatByteSize(3_000_000_000), "3.0 GB");
+    assert.strictEqual(formatByteSize(600_000_000_000), "600 GB");
+    assert.strictEqual(formatByteSize(60_598_160), "60.6 MB");
+    assert.strictEqual(formatByteSize(60_598_160_000), "60.6 GB");
+    assert.strictEqual(formatByteSize(60_598_160_000_000), "60.6 TB");
+  });
+});