fix(db): guard postgres init against invalid DATABASE_URL to prevent import-time crash#491
Conversation
…r malformed Co-authored-by: MightyPrytanis <219587333+MightyPrytanis@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an import-time crash caused by postgres.js throwing TypeError: Invalid URL when DATABASE_URL is malformed or whitespace-padded in CI. The module now validates and sanitizes the URL before attempting to create a postgres client, falling back to null gracefully.
Changes:
- Added
isValidPostgresUrl()helper to validate URL protocol before passing it topostgres() - Trimmed
DATABASE_URLto handle whitespace from CI secret injection - Wrapped
postgres()initialization intry/catchwith structured fallback branches
| console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.'); | ||
| } else if (!isValidPostgresUrl(dbUrl)) { | ||
| console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.'); |
There was a problem hiding this comment.
In the original code, the if (!dbUrl) block existed as part of the old ternary flow. Now with the new branching structure, this if block only logs a warning but does not explicitly assign client = null. While client is initialized as null on line 24 so the behavior is correct, the asymmetry is worth noting: the catch block on line 42 explicitly sets client = null, but neither the !dbUrl branch nor the !isValidPostgresUrl branch do. For clarity and defensive consistency, consider adding client = null; in both the !dbUrl and !isValidPostgresUrl branches.
| console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.'); | |
| } else if (!isValidPostgresUrl(dbUrl)) { | |
| console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.'); | |
| console.warn('[DB] WARNING: DATABASE_URL not set. Database operations will fail.'); | |
| client = null; | |
| } else if (!isValidPostgresUrl(dbUrl)) { | |
| console.warn('[DB] WARNING: DATABASE_URL is not a valid postgres URL. Database operations will fail.'); | |
| client = null; |
Cyrano/src/db.tscalledpostgres(dbUrl)wheneverDATABASE_URLwas truthy, with no URL validation. In CI, the secret value is malformed or whitespace-padded, causingpostgres.jsto throwTypeError: Invalid URLat import time — propagating through every module that importsdband crashing 15+ test files before any assertions run, dropping the quality gate pass rate below 85%.Changes
Cyrano/src/db.tsisValidPostgresUrl()— validates vianew URL(), accepts onlypostgres:/postgresql:protocols before touchingpostgres().trim()onDATABASE_URLto handle whitespace from copy-paste or CI secret injectionpostgres()init intry/catch; falls back tonulland logserror.message(not the URL)Modules that don't use the DB will import and run cleanly. DB-dependent paths will fail at query time with a clear runtime error rather than a silent import-time crash.
Original prompt
Create a PR to fix CI failures in failing job 66355029822 for PR #472 in MightyPrytanis/codebase.
Context
TypeError: Invalid URLoriginating from postgres.js parsingprocess.env.DATABASE_URL.parseUrl->Cyrano/src/db.ts:20:24db:Cyrano/src/services/hipaa-compliance.ts(imports db)Cyrano/src/tools/arkiver-mcp-tools.ts(imports db)Cyrano/src/routes/auth.ts(imports db).github/workflows/ci.yml, making this import-time crash occur in CI.Goal
Cyrano/src/db.tsabout not failing on import.Required changes
Update
Cyrano/src/db.ts:const dbUrl = process.env.DATABASE_URL?.trim();new URL(dbUrl)ensuring protocol ispostgres:orpostgresql:.postgres(dbUrl, options)in try/catch.client = nulland exportdbas null-ish (null as anyis acceptable for now) so imports succeed.Optionally (only if needed for type safety): export a helper like
isDbAvailable()orgetDbOrThrow()and update any obvious call sites that assume db exists at runtime; but do not refactor broadly.Run/update tests as needed so CI no longer fails due to invalid URL import-time crash.
Non-goals
Acceptance criteria
TypeError: Invalid URLon module import.The following is the prior conversation context from the user's chat exploration (may be truncated):
User: Please find a solution for failing job 66355029822. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.
Assistant: The failing job 66355029822 is ultimately failing because multiple test files crash at import-time with:
TypeError: Invalid URLpostgres’sparseUrl(...)Cyrano/src/db.ts:20db(e.g.Cyrano/src/services/hipaa-compliance.ts:7,Cyrano/src/tools/arkiver-mcp-tools.ts:17,Cyrano/src/routes/auth.ts:16).That import-time crash causes 15 test files to fail, which then trips the Quality Gates step (pass rate 73.21% < 85%).
Root cause
In
Cyrano/src/db.ts, the code callspostgres(dbUrl, ...)wheneverprocess.env.DATABASE_URLis “truthy”:In CI,
DATABASE_URLis being provided (.github/workflows/ci.ymlsets it fromsecrets.DATABASE_URL), but the value being passed into the runtime is not a valid URL forpostgres(the logs showERR_INVALID_URLwith redacted input). As soon as any module importsdb,postgres(dbUrl, ...)parses it and throws.Fix (recommended): make DB initialization safe in test/CI when URL is invalid
Update
Cyrano/src/db.tsto validate the URL (and/or catch initialization errors) and fall back tonullinstead of throwing during import. This preserves the “don’t fail on import” intent mentioned in comments.Patch suggestion (minimal, robust)