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

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

Merged
merged 6 commits into from
Feb 5, 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
25 changes: 25 additions & 0 deletions .changeset/nervous-bottles-switch.md
Original file line number Diff line number Diff line change
@@ -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
}
}
```
13 changes: 13 additions & 0 deletions packages/wrangler/src/__tests__/d1/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
95 changes: 54 additions & 41 deletions packages/wrangler/src/d1/execute.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
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";
Expand Down Expand Up @@ -104,50 +104,63 @@
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(

Check warning on line 126 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L126

Added line #L126 was not covered by tests
renderToString(
<Static items={response}>
{(result) => {

Check warning on line 129 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L129

Added line #L129 was not covered by tests
// batch results
if (!Array.isArray(result)) {
const { results, query } = result;

Check warning on line 132 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L132

Added line #L132 was not covered by tests

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 (

Check warning on line 136 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L135-L136

Added lines #L135 - L136 were not covered by tests
<>
{shortQuery ? <Text dimColor>{shortQuery}</Text> : null}

Check warning on line 138 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L138

Added line #L138 was not covered by tests
<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 {

Check warning on line 148 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L148

Added line #L148 was not covered by tests
// 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));

Check warning on line 151 in packages/wrangler/src/d1/execute.tsx

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/d1/execute.tsx#L150-L151

Added lines #L150 - L151 were not covered by tests
}
} 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;
}
}
};

Expand Down
14 changes: 14 additions & 0 deletions packages/wrangler/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
4 changes: 3 additions & 1 deletion packages/wrangler/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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")
Expand Down
Loading