From 0715f1f29f9791d233c13d998aab7b1e8a295102 Mon Sep 17 00:00:00 2001 From: Vasily Scherbenko Date: Sat, 1 Feb 2025 15:22:07 +0300 Subject: [PATCH 1/2] fix: can not convert object to primitive nonsence error message --- src/umzug.ts | 11 ++++++++--- test/umzug.test.ts | 7 +++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/umzug.ts b/src/umzug.ts index a520c325..baa8adcb 100644 --- a/src/umzug.ts +++ b/src/umzug.ts @@ -48,9 +48,14 @@ export class MigrationError extends errorCause.ErrorWithCause { } private static errorString(cause: unknown) { - return cause instanceof Error - ? `Original error: ${cause.message}` - : `Non-error value thrown. See info for full props: ${cause as string}` + if (cause instanceof Error) return `Original error: ${cause.message}` + const msg = 'Non-error value thrown. See info for full props' + try { + return `${msg}: ${String(cause)}` + } catch { + //Null prototype object 'cause' would end up here + return msg + } } } diff --git a/test/umzug.test.ts b/test/umzug.test.ts index 4e965b35..8c63eb8c 100644 --- a/test/umzug.test.ts +++ b/test/umzug.test.ts @@ -685,7 +685,10 @@ describe('alternate migration inputs', () => { async up() {}, async down() { - throw 'Some cryptic failure' + // This kind of error is thrown in MikroOrm migrations + const reallyCrypticFailure: any = Object.create(null) + reallyCrypticFailure.customText = 'Some cryptic failure' + throw reallyCrypticFailure }, }, { @@ -705,7 +708,7 @@ describe('alternate migration inputs', () => { ) await expect(umzug.down()).rejects.toThrowErrorMatchingInlineSnapshot( - `"Migration m1 (down) failed: Non-error value thrown. See info for full props: Some cryptic failure"`, + `"Migration m1 (down) failed: Non-error value thrown. See info for full props"`, ) }) From 8ad06d2a5d289f4d6f18dc1629aec936aea2d5f4 Mon Sep 17 00:00:00 2001 From: Vasily Scherbenko Date: Wed, 5 Feb 2025 12:58:32 +0300 Subject: [PATCH 2/2] fix: error message without try/catch --- src/umzug.ts | 9 +++------ test/umzug.test.ts | 43 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/umzug.ts b/src/umzug.ts index baa8adcb..21dc8379 100644 --- a/src/umzug.ts +++ b/src/umzug.ts @@ -50,12 +50,9 @@ export class MigrationError extends errorCause.ErrorWithCause { private static errorString(cause: unknown) { if (cause instanceof Error) return `Original error: ${cause.message}` const msg = 'Non-error value thrown. See info for full props' - try { - return `${msg}: ${String(cause)}` - } catch { - //Null prototype object 'cause' would end up here - return msg - } + if (typeof cause !== 'object' || (cause as {}).toString !== undefined) return `${msg}: ${String(cause)}` + //Null prototype object 'cause' would end up here + return msg } } diff --git a/test/umzug.test.ts b/test/umzug.test.ts index 8c63eb8c..80ac74e3 100644 --- a/test/umzug.test.ts +++ b/test/umzug.test.ts @@ -685,7 +685,40 @@ describe('alternate migration inputs', () => { async up() {}, async down() { - // This kind of error is thrown in MikroOrm migrations + throw 'Some cryptic failure' + }, + }, + { + name: 'm2', + + async up() { + throw 'Some cryptic failure' + }, + async down() {}, + }, + ], + logger: undefined, + }) + + await expect(umzug.up()).rejects.toThrowErrorMatchingInlineSnapshot( + `"Migration m2 (up) failed: Non-error value thrown. See info for full props: Some cryptic failure"`, + ) + + await expect(umzug.down()).rejects.toThrowErrorMatchingInlineSnapshot( + `"Migration m1 (down) failed: Non-error value thrown. See info for full props: Some cryptic failure"`, + ) + }) + + test('null-prototype throwables are wrapped helpfully', async () => { + // Errors derived from null prototype can not be printed with toString(), make sure we do not fail printing them + // This kind of error is thrown in MikroOrm migrations + const umzug = new Umzug({ + migrations: [ + { + name: 'm1', + async up() {}, + + async down() { const reallyCrypticFailure: any = Object.create(null) reallyCrypticFailure.customText = 'Some cryptic failure' throw reallyCrypticFailure @@ -695,7 +728,9 @@ describe('alternate migration inputs', () => { name: 'm2', async up() { - throw 'Some cryptic failure' + const reallyCrypticFailure: any = Object.create(null) + reallyCrypticFailure.customText = 'Some cryptic failure' + throw reallyCrypticFailure }, async down() {}, }, @@ -704,11 +739,11 @@ describe('alternate migration inputs', () => { }) await expect(umzug.up()).rejects.toThrowErrorMatchingInlineSnapshot( - `"Migration m2 (up) failed: Non-error value thrown. See info for full props: Some cryptic failure"`, + `"Migration m2 (up) failed: Non-error value thrown. See info for full props"`, ) await expect(umzug.down()).rejects.toThrowErrorMatchingInlineSnapshot( - `"Migration m1 (down) failed: Non-error value thrown. See info for full props"`, + `"Migration m1 (down) failed: Non-error value thrown. See info for full props"`, ) })