fix(d1): intercept and stringify errors thrown in --json mode (#4872)

* fix(d1): intercept and stringify errors thrown in --json mode * fix: add light parsing on d1 execute error message * add test * wip * implement JsonFriendlyFatalError * address PR feedback
cloudflare · Feb 5, 2024 · 5ef5606 · 5ef5606
1 parent 4c7031a
commit 5ef5606
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 42 deletions.
diff --git a/.changeset/nervous-bottles-switch.md b/.changeset/nervous-bottles-switch.md
@@ -0,0 +1,25 @@
+---
+"wrangler": patch
+---
+
+fix: intercept and stringify errors thrown by d1 execute in --json mode
+
+Prior to this PR, if a query threw an error when run in `wrangler d1 execute ... --json`, wrangler would swallow the error.
+
+This PR returns the error as JSON. For example, the invalid query `SELECT asdf;` now returns the following in JSON mode:
+
+```json
+{
+	"error": {
+		"text": "A request to the Cloudflare API (/accounts/xxxx/d1/database/xxxxxxx/query) failed.",
+		"notes": [
+			{
+				"text": "no such column: asdf at offset 7 [code: 7500]"
+			}
+		],
+		"kind": "error",
+		"name": "APIError",
+		"code": 7500
+	}
+}
+```
diff --git a/packages/wrangler/src/__tests__/d1/execute.test.ts b/packages/wrangler/src/__tests__/d1/execute.test.ts
@@ -50,6 +50,19 @@ describe("execute", () => {
 		).rejects.toThrowError(`Error: can't use --preview with --local`);
 	});
 
+	it("should reject the use of --preview with --local with --json", async () => {
+		setIsTTY(false);
+		writeWranglerToml({
+			d1_databases: [
+				{ binding: "DATABASE", database_name: "db", database_id: "xxxx" },
+			],
+		});
+
+		await expect(
+			runWrangler(`d1 execute db --command "select;" --local --preview --json`)
+		).rejects.toThrowError(`Error: can't use --preview with --local`);
+	});
+
 	it("should expect --local when using --persist-to", async () => {
 		setIsTTY(false);
 		writeWranglerToml({

diff --git a/packages/wrangler/src/d1/execute.tsx b/packages/wrangler/src/d1/execute.tsx
@@ -9,7 +9,7 @@ import { fetchResult } from "../cfetch";
 import { readConfig } from "../config";
 import { getLocalPersistencePath } from "../dev/get-local-persistence-path";
 import { confirm } from "../dialogs";
-import { UserError } from "../errors";
+import { JsonFriendlyFatalError, UserError } from "../errors";
 import { logger } from "../logger";
 import { readFileSync } from "../parse";
 import { readableRelative } from "../paths";
@@ -104,50 +104,63 @@ export const Handler = async (args: HandlerOptions): Promise<void> => {
 		return logger.error(`Error: can't provide both --command and --file.`);
 
 	const isInteractive = process.stdout.isTTY;
-	const response: QueryResult[] | null = await executeSql({
-		local,
-		config,
-		name: database,
-		shouldPrompt: isInteractive && !yes,
-		persistTo,
-		file,
-		command,
-		json,
-		preview,
-		batchSize,
-	});
+	try {
+		const response: QueryResult[] | null = await executeSql({
+			local,
+			config,
+			name: database,
+			shouldPrompt: isInteractive && !yes,
+			persistTo,
+			file,
+			command,
+			json,
+			preview,
+			batchSize,
+		});
 
-	// Early exit if prompt rejected
-	if (!response) return;
+		// Early exit if prompt rejected
+		if (!response) return;
 
-	if (isInteractive && !json) {
-		// Render table if single result
-		logger.log(
-			renderToString(
-				<Static items={response}>
-					{(result) => {
-						// batch results
-						if (!Array.isArray(result)) {
-							const { results, query } = result;
+		if (isInteractive && !json) {
+			// Render table if single result
+			logger.log(
+				renderToString(
+					<Static items={response}>
+						{(result) => {
+							// batch results
+							if (!Array.isArray(result)) {
+								const { results, query } = result;
 
-							if (Array.isArray(results) && results.length > 0) {
-								const shortQuery = shorten(query, 48);
-								return (
-									<>
-										{shortQuery ? <Text dimColor>{shortQuery}</Text> : null}
-										<Table data={results}></Table>
-									</>
-								);
+								if (Array.isArray(results) && results.length > 0) {
+									const shortQuery = shorten(query, 48);
+									return (
+										<>
+											{shortQuery ? <Text dimColor>{shortQuery}</Text> : null}
+											<Table data={results}></Table>
+										</>
+									);
+								}
 							}
-						}
-					}}
-				</Static>
-			)
-		);
-	} else {
-		// set loggerLevel back to what it was before to actually output the JSON in stdout
-		logger.loggerLevel = existingLogLevel;
-		logger.log(JSON.stringify(response, null, 2));
+						}}
+					</Static>
+				)
+			);
+		} else {
+			// set loggerLevel back to what it was before to actually output the JSON in stdout
+			logger.loggerLevel = existingLogLevel;
+			logger.log(JSON.stringify(response, null, 2));
+		}
+	} catch (error) {
+		if (json && error instanceof Error) {
+			logger.loggerLevel = existingLogLevel;
+			const messageToDisplay =
+				error.name === "APIError" ? error : { text: error.message };
+			throw new JsonFriendlyFatalError(
+				JSON.stringify({ error: messageToDisplay }, null, 2)
+			);
+		} else {
+			throw error;
+		}
 	}
 };
 

diff --git a/packages/wrangler/src/errors.ts b/packages/wrangler/src/errors.ts
@@ -24,3 +24,17 @@ export class FatalError extends UserError {
 		super(message);
 	}
 }
+
+/**
+ * JsonFriendlyFatalError is used to output JSON when wrangler crashes, useful for --json mode.
+ *
+ * To use, pass stringify'd json into the constructor like so:
+ * ```js
+ * throw new JsonFriendlyFatalError(JSON.stringify({ error: messageToDisplay });
+ * ```
+ */
+export class JsonFriendlyFatalError extends FatalError {
+	constructor(message?: string, readonly code?: number) {
+		super(message);
+	}
+}
diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts
@@ -35,7 +35,7 @@ import {
 import { devHandler, devOptions } from "./dev";
 import { workerNamespaceCommands } from "./dispatch-namespace";
 import { docsHandler, docsOptions } from "./docs";
-import { UserError } from "./errors";
+import { JsonFriendlyFatalError, UserError } from "./errors";
 import { generateHandler, generateOptions } from "./generate";
 import { hyperdrive } from "./hyperdrive/index";
 import { initHandler, initOptions } from "./init";
@@ -787,6 +787,8 @@ export async function main(argv: string[]): Promise<void> {
 				text: "\nIf you think this is a bug, please open an issue at: https://github.com/cloudflare/workers-sdk/issues/new/choose",
 			});
 			logger.log(formatMessage(e));
+		} else if (e instanceof JsonFriendlyFatalError) {
+			logger.log(e.message);
 		} else if (
 			e instanceof Error &&
 			e.message.includes("Raw mode is not supported on")