Skip to content

Commit a178bed

Browse files
committed
feat: friendly error when migration cycle failed
1 parent f275171 commit a178bed

File tree

4 files changed

+52
-9
lines changed

4 files changed

+52
-9
lines changed

src/datastore/event-requests.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,21 +119,28 @@ export async function* getRawEventRequests(
119119
}
120120
}
121121

122-
export async function databaseHasData(): Promise<boolean> {
122+
/**
123+
* Check the `pg_class` table for any data structures contained in the database. We will consider
124+
* any and all results here as "data" contained in the DB, since anything that is not a completely
125+
* empty DB could lead to strange errors when running the API. See:
126+
* https://www.postgresql.org/docs/current/catalog-pg-class.html
127+
* @returns `boolean` if the DB has data
128+
*/
129+
export async function databaseHasData(args?: {
130+
ignoreMigrationTables?: boolean;
131+
}): Promise<boolean> {
123132
const sql = await connectPostgres({
124133
usageName: 'contains-data-check',
125134
pgServer: PgServer.primary,
126135
});
127136
try {
128-
// Check the `pg_class` table for any data structures contained in the database. We will
129-
// consider any and all results here as "data" contained in the DB, since anything that is not a
130-
// completely empty DB could lead to strange errors when running the API. See:
131-
// https://www.postgresql.org/docs/current/catalog-pg-class.html
137+
const ignoreMigrationTables = args?.ignoreMigrationTables ?? false;
132138
const result = await sql<{ count: number }[]>`
133139
SELECT COUNT(*)
134140
FROM pg_class c
135141
JOIN pg_namespace s ON s.oid = c.relnamespace
136142
WHERE s.nspname = ${sql.options.connection.search_path}
143+
${ignoreMigrationTables ? sql`AND c.relname NOT LIKE 'pgmigrations%'` : sql``}
137144
`;
138145
return result.count > 0 && result[0].count > 0;
139146
} catch (error: any) {

src/datastore/migrations.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Client } from 'pg';
44
import { APP_DIR, isDevEnv, isTestEnv, logError, logger } from '../helpers';
55
import { getPgClientConfig, PgClientConfig } from './connection-legacy';
66
import { connectPostgres, PgServer } from './connection';
7+
import { databaseHasData } from './event-requests';
78

89
const MIGRATIONS_TABLE = 'pgmigrations';
910
const MIGRATIONS_DIR = path.join(APP_DIR, 'migrations');
@@ -53,10 +54,14 @@ export async function runMigrations(
5354
export async function cycleMigrations(opts?: {
5455
// Bypass the NODE_ENV check when performing a "down" migration which irreversibly drops data.
5556
dangerousAllowDataLoss?: boolean;
57+
checkForEmptyData?: boolean;
5658
}): Promise<void> {
5759
const clientConfig = getPgClientConfig({ usageName: 'cycle-migrations' });
5860

5961
await runMigrations(clientConfig, 'down', opts);
62+
if (opts?.checkForEmptyData && (await databaseHasData({ ignoreMigrationTables: true }))) {
63+
throw new Error('Migration down process did not completely remove DB tables');
64+
}
6065
await runMigrations(clientConfig, 'up', opts);
6166
}
6267

src/event-replay/event-replay.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,14 @@ export async function importEventsFromTsv(
9898
await dangerousDropAllTables({ acknowledgePotentialCatastrophicConsequences: 'yes' });
9999
}
100100

101-
// This performs a "migration down" which drops the tables, then re-creates them.
102-
// If there's a breaking change in the migration files, this will throw, and the pg database needs wiped manually,
103-
// or the `--force` option can be used.
104-
await cycleMigrations({ dangerousAllowDataLoss: true });
101+
try {
102+
await cycleMigrations({ dangerousAllowDataLoss: true, checkForEmptyData: true });
103+
} catch (error) {
104+
logger.error(error);
105+
throw new Error(
106+
`DB migration cycle failed, possibly due to an incompatible API version upgrade. Add --wipe-db --force or perform a manual DB wipe before importing.`
107+
);
108+
}
105109

106110
// Look for the TSV's block height and determine the prunable block window.
107111
const tsvBlockHeight = await findTsvBlockHeight(resolvedFilePath);

src/tests-event-replay/import-export-tests.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,42 @@ describe('import/export tests', () => {
5757
}
5858
});
5959

60+
test('import with db wipe options', async () => {
61+
// Migrate first so we have some data.
62+
const clientConfig = getPgClientConfig({ usageName: 'cycle-migrations' });
63+
await runMigrations(clientConfig, 'up', {});
64+
await expect(
65+
importEventsFromTsv('src/tests-event-replay/tsv/mocknet.tsv', 'archival', false, false)
66+
).rejects.toThrowError('contains existing data');
67+
68+
// Create strange table
69+
await db.sql`CREATE TABLE IF NOT EXISTS test (a varchar(10))`;
70+
await expect(
71+
importEventsFromTsv('src/tests-event-replay/tsv/mocknet.tsv', 'archival', true, false)
72+
).rejects.toThrowError('migration cycle failed');
73+
74+
// Force and test
75+
await expect(
76+
importEventsFromTsv('src/tests-event-replay/tsv/mocknet.tsv', 'archival', true, true)
77+
).resolves.not.toThrow();
78+
});
79+
6080
test('db contains data', async () => {
6181
const clientConfig = getPgClientConfig({ usageName: 'cycle-migrations' });
6282
await runMigrations(clientConfig, 'up', {});
6383

6484
// Having tables counts as having data as this may change across major versions.
6585
await expect(databaseHasData()).resolves.toBe(true);
6686

87+
// Dropping all tables removes everything.
6788
await dangerousDropAllTables({ acknowledgePotentialCatastrophicConsequences: 'yes' });
6889
await expect(databaseHasData()).resolves.toBe(false);
90+
91+
// Cycling migrations leaves the `pgmigrations` table.
92+
await runMigrations(clientConfig, 'up', {});
93+
await runMigrations(clientConfig, 'down', {});
94+
await expect(databaseHasData()).resolves.toBe(true);
95+
await expect(databaseHasData({ ignoreMigrationTables: true })).resolves.toBe(false);
6996
});
7097

7198
afterEach(async () => {

0 commit comments

Comments
 (0)