fix(internals): fall back to raw stderr when schema engine error has no message

packages/internals/src/__tests__/schemaEngineCommands.test.ts

@@ -3,7 +3,15 @@ import tempy from 'tempy'
 import { describe, expect, test, vi } from 'vitest'
 
 import { credentialsToUri, uriToCredentials } from '../convertCredentials'
-import { canConnectToDatabase, createDatabase, dropDatabase, execaCommand } from '../schemaEngineCommands'
+import {
+  canConnectToDatabase,
+  createDatabase,
+  dropDatabase,
+  execaCommand,
+  formatSchemaEngineError,
+  parseJsonFromStderr,
+  type SchemaEngineLogLine,
+} from '../schemaEngineCommands'
 
 if (process.env.CI) {
   // 5s is often not enough for the "postgresql - create database" test on macOS CI.
@@ -29,6 +37,47 @@ describe('execaCommand', () => {
   })
 })
 
+describe('formatSchemaEngineError', () => {
+  const log = (message: string): SchemaEngineLogLine => ({
+    timestamp: '2021-06-11T15:35:34.084486+00:00',
+    level: 'ERROR',
+    target: 'schema_engine::logger',
+    fields: { message },
+  })
+
+  test('joins messages from multiple log lines', () => {
+    expect(formatSchemaEngineError([log('first'), log('second')], 'raw stderr')).toBe('first\nsecond')
+  })
+
+  test('falls back to the raw stderr when no log line has a message', () => {
+    // e.g. when parseJsonFromStderr's `.slice(1)` drops the engine's only stderr line,
+    // leaving no logs to extract a message from.
+    expect(formatSchemaEngineError([], 'the only line of stderr, with the real error')).toBe(
+      'the only line of stderr, with the real error',
+    )
+  })
+
+  test('falls back to the raw stderr when log lines have empty messages', () => {
+    expect(formatSchemaEngineError([log('')], 'raw stderr')).toBe('raw stderr')
+  })
+
+  test('falls back to the raw stderr when log lines have whitespace-only messages', () => {
+    expect(formatSchemaEngineError([log('   ')], 'raw stderr')).toBe('raw stderr')
+  })
+})
+
+describe('parseJsonFromStderr', () => {
+  test('does not throw on a single-line stderr with a trailing newline', () => {
+    // stderr.split(/\r?\n/).slice(1) on "real error\n" leaves [''], which used to
+    // reach JSON.parse('') and throw before formatSchemaEngineError's fallback ever ran.
+    expect(parseJsonFromStderr('real error\n')).toEqual([])
+  })
+
+  test('does not throw on a single-line stderr with no trailing newline', () => {
+    expect(parseJsonFromStderr('real error')).toEqual([])
+  })
+})
+
 describe('canConnectToDatabase', () => {
   test('sqlite - can', async () => {
     await expect(canConnectToDatabase('file:./introspection/blog.db', __dirname)).resolves.toEqual(true)

packages/internals/src/schemaEngineCommands.ts

@@ -46,9 +46,12 @@ export interface ConnectionError {
   code: DatabaseErrorCodes
 }
 
-function parseJsonFromStderr(stderr: string): SchemaEngineLogLine[] {
+export function parseJsonFromStderr(stderr: string): SchemaEngineLogLine[] {
   // split by new line
-  const lines = stderr.split(/\r?\n/).slice(1) // Remove first element
+  const lines = stderr
+    .split(/\r?\n/)
+    .slice(1) // Remove first element
+    .filter((line) => line.trim() !== '') // A trailing newline leaves a blank line that isn't valid JSON
   const logs: any = []
 
   for (const line of lines) {
@@ -64,6 +67,18 @@ function parseJsonFromStderr(stderr: string): SchemaEngineLogLine[] {
   return logs
 }
 
+/**
+ * `parseJsonFromStderr` drops the engine's first stderr line as a discardable
+ * preamble. When the engine only emits that one line for a given failure, the
+ * only line with real information is dropped, `logs` ends up empty, and this
+ * used to produce a bare "Schema engine error:" with nothing after it. Fall
+ * back to the raw stderr so the error always carries some diagnostic content.
+ */
+export function formatSchemaEngineError(logs: SchemaEngineLogLine[], stderr: string): string {
+  const messages = logs.map((log) => log.fields.message).filter((message) => Boolean(message?.trim()))
+  return messages.length > 0 ? messages.join('\n') : stderr
+}
+
 // could be refactored with engines using JSON RPC instead and just passing the schema
 export async function canConnectToDatabase(
   connectionString: string,
@@ -94,7 +109,7 @@ export async function canConnectToDatabase(
           message: error.fields.message,
         }
       } else {
-        throw new Error(`Schema engine error:\n${logs.map((log) => log.fields.message).join('\n')}`)
+        throw new Error(`Schema engine error:\n${formatSchemaEngineError(logs, e.stderr)}`)
       }
     } else {
       throw new Error(`Schema engine exited. ${_e}`)
@@ -132,7 +147,7 @@ export async function createDatabase(connectionString: string, cwd = process.cwd
       if (error && error.fields.error_code && error.fields.message) {
         throw new Error(`${error.fields.error_code}: ${error.fields.message}`)
       } else {
-        throw new Error(`Schema engine error:\n${logs.map((log) => log.fields.message).join('\n')}`)
+        throw new Error(`Schema engine error:\n${formatSchemaEngineError(logs, e.stderr)}`)
       }
     } else {
       throw new Error(`Schema engine exited. ${_e}`)
@@ -158,7 +173,7 @@ export async function dropDatabase(connectionString: string, cwd = process.cwd()
     if (e.stderr) {
       const logs = parseJsonFromStderr(e.stderr)
 
-      throw new Error(`Schema engine error:\n${logs.map((log) => log.fields.message).join('\n')}`)
+      throw new Error(`Schema engine error:\n${formatSchemaEngineError(logs, String(e.stderr))}`)
     } else {
       throw new Error(`Schema engine exited. ${e}`)
     }