Skip to content

Commit

Permalink
fix: do not report D1 user errors to Sentry (#6192)
Browse files Browse the repository at this point in the history
  • Loading branch information
petebacondarwin committed Jul 4, 2024
1 parent 062e414 commit b879ce4
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/eighty-donuts-wonder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: do not report D1 user errors to Sentry
38 changes: 37 additions & 1 deletion packages/wrangler/src/__tests__/d1/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import { runWrangler } from "../helpers/run-wrangler";
import writeWranglerToml from "../helpers/write-wrangler-toml";

describe("execute", () => {
mockConsoleMethods();
const std = mockConsoleMethods();
runInTempDir();
const { setIsTTY } = useMockIsTTY();

// We turn on SENTRY reporting so that we can prove that Wrangler will not attempt to report user errors.
useSentry();

it("should require login when running against prod", async () => {
setIsTTY(false);
writeWranglerToml({
Expand Down Expand Up @@ -39,6 +42,26 @@ describe("execute", () => {
);
});

it("should reject use of both --command and --file", async () => {
setIsTTY(false);
writeWranglerToml({
d1_databases: [
{ binding: "DATABASE", database_name: "db", database_id: "xxxx" },
],
});

await expect(
runWrangler(
"d1 execute db --command='SELECT * FROM CUSTOMERS' --file=query.sql"
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Error: can't provide both --command and --file.]`
);
expect(std.out).not.toMatch(
"Would you like to report this error to Cloudflare?"
);
});

it("should reject the use of --remote with --local", async () => {
setIsTTY(false);
writeWranglerToml({
Expand Down Expand Up @@ -115,3 +138,16 @@ describe("execute", () => {
);
});
});

function useSentry() {
// @ts-expect-error TS does not know about SENTRY_DSN
const oldSentryDsn = global.SENTRY_DSN;
beforeEach(() => {
// @ts-expect-error TS does not know about SENTRY_DSN
global.SENTRY_DSN = "FAKE_VALUE";
});
afterEach(() => {
// @ts-expect-error TS does not know about SENTRY_DSN
global.SENTRY_DSN = oldSentryDsn;
});
}
36 changes: 24 additions & 12 deletions packages/wrangler/src/d1/execute.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { fetchResult } from "../cfetch";
import { readConfig } from "../config";
import { getLocalPersistencePath } from "../dev/get-local-persistence-path";
import { confirm } from "../dialogs";
import { JsonFriendlyFatalError, UserError } from "../errors";
import { createFatalError, JsonFriendlyFatalError, UserError } from "../errors";
import { logger } from "../logger";
import { APIError, readFileSync } from "../parse";
import { readableRelative } from "../paths";
Expand Down Expand Up @@ -117,7 +117,10 @@ export const Handler = async (args: HandlerOptions): Promise<void> => {
const config = readConfig(args.config, args);

if (file && command) {
return logger.error(`Error: can't provide both --command and --file.`);
throw createFatalError(
`Error: can't provide both --command and --file.`,
json
);
}

const isInteractive = process.stdout.isTTY;
Expand Down Expand Up @@ -552,17 +555,26 @@ async function d1ApiPost<T>(
action: string,
body: unknown
) {
return await fetchResult<T>(
`/accounts/${accountId}/d1/database/${db.uuid}/${action}`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
...(db.internal_env ? { "x-d1-internal-env": db.internal_env } : {}),
},
body: JSON.stringify(body),
try {
return await fetchResult<T>(
`/accounts/${accountId}/d1/database/${db.uuid}/${action}`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
...(db.internal_env ? { "x-d1-internal-env": db.internal_env } : {}),
},
body: JSON.stringify(body),
}
);
} catch (x) {
if (x instanceof APIError) {
// API Errors here are most likely to be user errors - e.g. invalid SQL.
// So we don't want to report those to Sentry.
x.preventReport();
}
);
throw x;
}
}

function logResult(r: QueryResult | QueryResult[]) {
Expand Down

0 comments on commit b879ce4

Please sign in to comment.