From b5ed2c882ff87be216d693e83caed80b6752c6e2 Mon Sep 17 00:00:00 2001 From: Charlie Lamb Date: Tue, 21 Apr 2026 12:04:26 +0100 Subject: [PATCH] chore: code review comments --- server/src/db/initDrizzle.ts | 6 ++--- .../internal/migrations/MigrationService.ts | 5 +--- server/tests/infra/criticalDbTimeout.test.ts | 11 ++------- .../crud/customers/get-customer.test.ts | 24 ++++++++++++------- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/server/src/db/initDrizzle.ts b/server/src/db/initDrizzle.ts index 97292e8af..02b852f1c 100644 --- a/server/src/db/initDrizzle.ts +++ b/server/src/db/initDrizzle.ts @@ -47,7 +47,7 @@ export const initDrizzle = ({ maxConnections?: number; replica?: boolean; /** Connect timeout in seconds */ - connectTimeout?: number; + connectTimeout?: number | null; poolConfig?: PoolConfig; } = {}) => { const dbUrl = @@ -59,7 +59,7 @@ export const initDrizzle = ({ ...poolConfig, max: maxConnections, connectionTimeoutMillis: - connectTimeout === undefined ? undefined : connectTimeout * 1000, + connectTimeout === null ? undefined : connectTimeout * 1000, }); const drizzleDb = drizzle(client, { schema }); @@ -98,7 +98,7 @@ export const { db: dbGeneral, client: clientGeneral } = initDrizzle({ // -- Replica pool: used as fallback when primary is degraded -- // Only created if DATABASE_REPLICA_URL is configured. const replicaResult = process.env.DATABASE_REPLICA_URL - ? initDrizzle({ replica: true, maxConnections: 5, connectTimeout: undefined }) + ? initDrizzle({ replica: true, maxConnections: 5, connectTimeout: null }) : null; export const dbReplica = replicaResult?.db ?? null; export const clientReplica = replicaResult?.client ?? null; diff --git a/server/src/internal/migrations/MigrationService.ts b/server/src/internal/migrations/MigrationService.ts index 83ffb3867..e4e7db7c2 100644 --- a/server/src/internal/migrations/MigrationService.ts +++ b/server/src/internal/migrations/MigrationService.ts @@ -59,10 +59,7 @@ export class MigrationService { const lockResult = await txDb.execute<{ acquired: boolean }>( sql`SELECT pg_try_advisory_xact_lock(${lockId}) as acquired`, ); - const lockRows = Array.isArray(lockResult) - ? lockResult - : (lockResult as any).rows; - const lockAcquired = lockRows[0]?.acquired; + const lockAcquired = lockResult[0]?.acquired; if (!lockAcquired) { throw new RecaseError({ diff --git a/server/tests/infra/criticalDbTimeout.test.ts b/server/tests/infra/criticalDbTimeout.test.ts index 5dc974c5b..76352933d 100644 --- a/server/tests/infra/criticalDbTimeout.test.ts +++ b/server/tests/infra/criticalDbTimeout.test.ts @@ -1,9 +1,8 @@ import net from "node:net"; -import { afterAll, expect, test } from "bun:test"; +import { expect, test } from "bun:test"; import { sql } from "drizzle-orm"; import { assertNotProductionDb } from "@/db/dbUtils.js"; import { - client, clientCritical, dbCritical, dbGeneral, @@ -55,9 +54,7 @@ test("db connect timeout fails fast when postgres accepts tcp but never responds const startedAt = Date.now(); try { - await expect(deadClient.query("SELECT 1")).rejects.toThrow( - /connection timeout/i, - ); + await expect(deadClient.query("SELECT 1")).rejects.toThrow(/timeout/i); expect(Date.now() - startedAt).toBeLessThan(2_000); } finally { @@ -68,7 +65,3 @@ test("db connect timeout fails fast when postgres accepts tcp but never responds }); } }, 5_000); - -afterAll(async () => { - await Promise.all([client.end(), clientCritical.end()]); -}); diff --git a/server/tests/integration/crud/customers/get-customer.test.ts b/server/tests/integration/crud/customers/get-customer.test.ts index 1bff9b672..7ccd5a624 100644 --- a/server/tests/integration/crud/customers/get-customer.test.ts +++ b/server/tests/integration/crud/customers/get-customer.test.ts @@ -60,9 +60,11 @@ test.concurrent(`${chalk.yellowBright("get-customer: multi-entity customer retur const cusV1 = await autumnV1.customers.get(customerId); ApiCustomerV3Schema.parse(cusV1); expect(cusV1.products.length).toBeGreaterThan(0); - const cusV1Prod = cusV1.products[0]; - expect(cusV1Prod.current_period_start).toBeNumber(); - expect(cusV1Prod.current_period_end).toBeNumber(); + const cusV1Prod = cusV1.products.find( + (product) => product.id === cusLevelProd.id, + ); + expect(cusV1Prod?.current_period_start).toBeNumber(); + expect(cusV1Prod?.current_period_end).toBeNumber(); expect(cusV1.features[TestFeature.Messages]).toMatchObject({ id: TestFeature.Messages, balance: 90, @@ -79,9 +81,11 @@ test.concurrent(`${chalk.yellowBright("get-customer: multi-entity customer retur }); ApiCustomerV5Schema.parse(cusV2_1); expect(cusV2_1.subscriptions.length).toBeGreaterThan(0); - const cusV2_1Sub = cusV2_1.subscriptions[0]; - expect(cusV2_1Sub.current_period_start).toBeNumber(); - expect(cusV2_1Sub.current_period_end).toBeNumber(); + const cusV2_1Sub = cusV2_1.subscriptions.find( + (subscription) => subscription.plan_id === cusLevelProd.id, + ); + expect(cusV2_1Sub?.current_period_start).toBeNumber(); + expect(cusV2_1Sub?.current_period_end).toBeNumber(); expectFlagCorrect({ customer: cusV2_1, featureId: TestFeature.Dashboard, @@ -103,9 +107,11 @@ test.concurrent(`${chalk.yellowBright("get-customer: multi-entity customer retur }); ApiCustomerV5Schema.parse(cusV2_2); expect(cusV2_2.subscriptions.length).toBeGreaterThan(0); - const cusV2_2Sub = cusV2_2.subscriptions[0]; - expect(cusV2_2Sub.current_period_start).toBeNumber(); - expect(cusV2_2Sub.current_period_end).toBeNumber(); + const cusV2_2Sub = cusV2_2.subscriptions.find( + (subscription) => subscription.plan_id === cusLevelProd.id, + ); + expect(cusV2_2Sub?.current_period_start).toBeNumber(); + expect(cusV2_2Sub?.current_period_end).toBeNumber(); expectFlagCorrect({ customer: cusV2_2, featureId: TestFeature.Dashboard,