[wrangler]: fix missing error on failed d1 migrations (#3124)

* [wrangler]: fix missing error on failed d1 migrations * apply PR feedback * Update apply.tsx * improve logging during migration apply * Update execute.tsx * Update .changeset/lemon-garlics-provide.md --------- Co-authored-by: Max Rozen <3822106+rozenmd@users.noreply.github.com>
cloudflare · May 30, 2023 · 2956c31 · 2956c31
1 parent f62d512
commit 2956c31
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 23 deletions.
diff --git a/.changeset/lemon-garlics-provide.md b/.changeset/lemon-garlics-provide.md
@@ -0,0 +1,9 @@
+---
+"wrangler": patch
+---
+
+fix: failed d1 migrations not treated as errors
+
+This PR teaches wrangler to return a non-success exit code when a set of migrations fails.
+
+It also cleans up `wrangler d1 migrations apply` output significantly, to only log information relevant to migrations.
diff --git a/packages/wrangler/src/d1/execute.tsx b/packages/wrangler/src/d1/execute.tsx
@@ -178,6 +178,11 @@ export async function executeSql({
 	preview: boolean | undefined;
 	batchSize: number;
 }) {
+	const existingLogLevel = logger.loggerLevel;
+	if (json) {
+		// set loggerLevel to error to avoid logs appearing in JSON output
+		logger.loggerLevel = "error";
+	}
 	const sql = file ? readFileSync(file) : command;
 	if (!sql) throw new Error(`Error: must provide --command or --file.`);
 	if (preview && local)
@@ -195,8 +200,7 @@ export async function executeSql({
 			);
 		}
 	}
-
-	return local
+	const result = local
 		? await executeLocally({
 				config,
 				name,
@@ -213,6 +217,8 @@ export async function executeSql({
 				json,
 				preview,
 		  });
+	if (json) logger.loggerLevel = existingLogLevel;
+	return result;
 }
 
 async function executeLocally({

diff --git a/packages/wrangler/src/d1/migrations/apply.tsx b/packages/wrangler/src/d1/migrations/apply.tsx
@@ -101,13 +101,13 @@ export const ApplyHandler = withConfig<ApplyHandlerOptions>(
 		)
 			.map((migration) => {
 				return {
-					Name: migration,
-					Status: "🕒️",
+					name: migration,
+					status: "🕒️",
 				};
 			})
 			.sort((a, b) => {
-				const migrationNumberA = parseInt(a.Name.split("_")[0]);
-				const migrationNumberB = parseInt(b.Name.split("_")[0]);
+				const migrationNumberA = parseInt(a.name.split("_")[0]);
+				const migrationNumberB = parseInt(b.name.split("_")[0]);
 				if (migrationNumberA < migrationNumberB) {
 					return -1;
 				}
@@ -127,7 +127,7 @@ export const ApplyHandler = withConfig<ApplyHandlerOptions>(
 			renderToString(
 				<Box flexDirection="column">
 					<Text>Migrations to be applied:</Text>
-					<Table data={unappliedMigrations} columns={["Name"]}></Table>
+					<Table data={unappliedMigrations} columns={["name"]}></Table>
 				</Box>
 			)
 		);
@@ -150,12 +150,12 @@ Your database may not be available to serve requests during the migration, conti
 
 		for (const migration of unappliedMigrations) {
 			let query = fs.readFileSync(
-				`${migrationsPath}/${migration.Name}`,
+				`${migrationsPath}/${migration.name}`,
 				"utf8"
 			);
 			query += `
 								INSERT INTO ${migrationsTableName} (name)
-								values ('${migration.Name}');
+								values ('${migration.name}');
 						`;
 
 			let success = true;
@@ -203,33 +203,36 @@ Your database may not be available to serve requests during the migration, conti
 				];
 			}
 
-			migration.Status = success ? "✅" : "❌";
+			migration.status = success ? "✅" : "❌";
 
 			logger.log(
 				renderToString(
 					<Box flexDirection="column">
-						<Table
-							data={unappliedMigrations}
-							columns={["Name", "Status"]}
-						></Table>
+						<Table data={unappliedMigrations} columns={["name", "status"]} />
 						{errorNotes.length > 0 && (
 							<Box flexDirection="column">
 								<Text>&nbsp;</Text>
 								<Text>
-									❌ Migration {migration.Name} failed with following Errors
+									❌ Migration {migration.name}{" "}
+									{errorNotes.length > 0
+										? "failed with the following errors:"
+										: ""}
 								</Text>
-								<Table
-									data={errorNotes.map((err) => {
-										return { Error: err };
-									})}
-								></Table>
 							</Box>
 						)}
 					</Box>
 				)
 			);
 
-			if (errorNotes.length > 0) return;
+			if (errorNotes.length > 0) {
+				throw new Error(
+					errorNotes
+						.map((err) => {
+							return err;
+						})
+						.join("\n")
+				);
+			}
 		}
 	}
 );
diff --git a/packages/wrangler/src/d1/migrations/helpers.ts b/packages/wrangler/src/d1/migrations/helpers.ts
@@ -98,7 +98,7 @@ const listAppliedMigrations = async (
 		FROM ${migrationsTableName}
 		ORDER BY id`,
 		file: undefined,
-		json: undefined,
+		json: true,
 		preview,
 		batchSize: DEFAULT_BATCH_SIZE,
 	});
@@ -164,7 +164,7 @@ export const initMigrationsTable = async ({
 		applied_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL
 );`,
 		file: undefined,
-		json: undefined,
+		json: true,
 		preview,
 		batchSize: DEFAULT_BATCH_SIZE,
 	});