Skip to content

Commit bea86f8

Browse files
authored
Merge pull request #8178 from BitGo/WP-7883
feat(express): improve typed routes validation error messages
2 parents b3ee986 + 29d9098 commit bea86f8

15 files changed

Lines changed: 321 additions & 69 deletions

modules/express/src/errors.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,13 @@ export class BitGoExpressError extends BitGoJsError {
5959
Object.setPrototypeOf(this, BitGoExpressError.prototype);
6060
}
6161
}
62+
63+
export class ValidationError extends BitGoJsError {
64+
public readonly status = 400;
65+
public override readonly name = 'ValidationError';
66+
67+
public constructor(message: string) {
68+
super(message);
69+
Object.setPrototypeOf(this, ValidationError.prototype);
70+
}
71+
}
Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,23 @@
11
import { createRouter, WrappedRouter } from '@api-ts/typed-express-router';
22

33
import { ExpressApi } from './api';
4+
import { createValidationError } from './utils';
5+
6+
const { version: bitgoJsVersion } = require('bitgo/package.json');
7+
const { version: bitgoExpressVersion } = require('../../package.json');
48

59
export default function (): WrappedRouter<ExpressApi> {
6-
const router: WrappedRouter<ExpressApi> = createRouter(ExpressApi);
10+
const router: WrappedRouter<ExpressApi> = createRouter(ExpressApi, {
11+
decodeErrorFormatter: (errors) => {
12+
const err = createValidationError(errors);
13+
return {
14+
error: err.message,
15+
message: err.message,
16+
name: err.name,
17+
bitgoJsVersion,
18+
bitgoExpressVersion,
19+
};
20+
},
21+
});
722
return router;
823
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import * as t from 'io-ts';
2+
3+
import { ValidationError } from '../errors';
4+
5+
/**
6+
* Formats io-ts validation errors into clear, human-readable messages.
7+
*/
8+
export function formatValidationErrors(errors: t.Errors): string {
9+
const seen = new Set<string>();
10+
const messages: string[] = [];
11+
12+
for (const error of errors) {
13+
// Build field path, filtering out internal keys
14+
const path = error.context
15+
.map((c) => c.key)
16+
.filter((key) => key && !/^\d+$/.test(key) && key !== 'body')
17+
.join('.');
18+
19+
if (!path || seen.has(path)) continue;
20+
seen.add(path);
21+
22+
const expected = error.context[error.context.length - 1]?.type.name;
23+
if (expected === 'undefined') continue;
24+
25+
if (error.value === undefined) {
26+
messages.push(`Missing required field '${path}'`);
27+
} else {
28+
const value = typeof error.value === 'object' ? JSON.stringify(error.value) : String(error.value);
29+
messages.push(`Invalid value for '${path}': expected ${expected}, got '${value}'`);
30+
}
31+
}
32+
33+
return messages.join('. ') + (messages.length ? '.' : '');
34+
}
35+
36+
/**
37+
* Creates a ValidationError from io-ts validation errors.
38+
*/
39+
export function createValidationError(errors: t.Errors): ValidationError {
40+
return new ValidationError(formatValidationErrors(errors));
41+
}

modules/express/test/unit/typedRoutes/acceptShare.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,8 @@ describe('AcceptShare codec tests', function () {
348348
.send(requestBody);
349349

350350
assert.strictEqual(result.status, 400);
351-
assert.ok(Array.isArray(result.body));
352-
assert.ok(result.body.length > 0);
351+
assert.ok(result.body.error);
352+
assert.ok(result.body.error.length > 0);
353353
});
354354

355355
it('should handle acceptShare method throwing error', async function () {

modules/express/test/unit/typedRoutes/expressWalletUpdate.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
322322
});
323323

324324
res.status.should.equal(400);
325-
res.body.should.be.an.Array();
326-
res.body[0].should.match(/signerHost/);
325+
res.body.should.have.property('error');
326+
res.body.error.should.match(/signerHost/);
327327
});
328328

329329
it('should return 400 when signerTlsCert is missing', async function () {
@@ -337,8 +337,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
337337
});
338338

339339
res.status.should.equal(400);
340-
res.body.should.be.an.Array();
341-
res.body[0].should.match(/signerTlsCert/);
340+
res.body.should.have.property('error');
341+
res.body.error.should.match(/signerTlsCert/);
342342
});
343343

344344
it('should return 400 when passphrase is missing', async function () {
@@ -352,8 +352,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
352352
});
353353

354354
res.status.should.equal(400);
355-
res.body.should.be.an.Array();
356-
res.body[0].should.match(/passphrase/);
355+
res.body.should.have.property('error');
356+
res.body.error.should.match(/passphrase/);
357357
});
358358

359359
it('should return 400 when signerHost has invalid type', async function () {
@@ -367,8 +367,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
367367
});
368368

369369
res.status.should.equal(400);
370-
res.body.should.be.an.Array();
371-
res.body[0].should.match(/signerHost/);
370+
res.body.should.have.property('error');
371+
res.body.error.should.match(/signerHost/);
372372
});
373373

374374
it('should return 400 when signerTlsCert has invalid type', async function () {
@@ -382,8 +382,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
382382
});
383383

384384
res.status.should.equal(400);
385-
res.body.should.be.an.Array();
386-
res.body[0].should.match(/signerTlsCert/);
385+
res.body.should.have.property('error');
386+
res.body.error.should.match(/signerTlsCert/);
387387
});
388388

389389
it('should return 400 when passphrase has invalid type', async function () {
@@ -397,8 +397,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
397397
});
398398

399399
res.status.should.equal(400);
400-
res.body.should.be.an.Array();
401-
res.body[0].should.match(/passphrase/);
400+
res.body.should.have.property('error');
401+
res.body.error.should.match(/passphrase/);
402402
});
403403

404404
it('should return 400 when signerMacaroon has invalid type', async function () {
@@ -413,8 +413,8 @@ describe('Express Wallet Update Typed Routes Tests', function () {
413413
});
414414

415415
res.status.should.equal(400);
416-
res.body.should.be.an.Array();
417-
res.body[0].should.match(/signerMacaroon/);
416+
res.body.should.have.property('error');
417+
res.body.error.should.match(/signerMacaroon/);
418418
});
419419
});
420420

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import * as assert from 'assert';
2+
import * as t from 'io-ts';
3+
4+
import { formatValidationErrors } from '../../../src/typedRoutes/utils';
5+
6+
describe('formatValidationErrors', function () {
7+
it('should format missing required field', function () {
8+
const errors: t.Errors = [{ value: undefined, context: [{ key: 'name', type: t.string }] }];
9+
assert.strictEqual(formatValidationErrors(errors), "Missing required field 'name'.");
10+
});
11+
12+
it('should format wrong type', function () {
13+
const errors: t.Errors = [{ value: 123, context: [{ key: 'field', type: t.string }] }];
14+
assert.strictEqual(formatValidationErrors(errors), "Invalid value for 'field': expected string, got '123'.");
15+
});
16+
17+
it('should format nested paths', function () {
18+
const errors: t.Errors = [
19+
{
20+
value: 123,
21+
context: [
22+
{ key: 'memo', type: t.object },
23+
{ key: 'type', type: t.string },
24+
],
25+
},
26+
];
27+
assert.strictEqual(formatValidationErrors(errors), "Invalid value for 'memo.type': expected string, got '123'.");
28+
});
29+
30+
it('should filter numeric indices', function () {
31+
const errors: t.Errors = [
32+
{
33+
value: 'x',
34+
context: [
35+
{ key: 'recipients', type: t.array(t.unknown) },
36+
{ key: '0', type: t.object },
37+
{ key: 'amount', type: t.number },
38+
],
39+
},
40+
];
41+
assert.strictEqual(
42+
formatValidationErrors(errors),
43+
"Invalid value for 'recipients.amount': expected number, got 'x'."
44+
);
45+
});
46+
47+
it('should filter body from path', function () {
48+
const errors: t.Errors = [
49+
{
50+
value: 123,
51+
context: [
52+
{ key: 'body', type: t.object },
53+
{ key: 'name', type: t.string },
54+
],
55+
},
56+
];
57+
assert.strictEqual(formatValidationErrors(errors), "Invalid value for 'name': expected string, got '123'.");
58+
});
59+
60+
it('should skip undefined type errors', function () {
61+
const errors: t.Errors = [{ value: 123, context: [{ key: 'optional', type: t.undefined }] }];
62+
assert.strictEqual(formatValidationErrors(errors), '');
63+
});
64+
65+
it('should return empty string for empty errors', function () {
66+
assert.strictEqual(formatValidationErrors([]), '');
67+
});
68+
69+
it('should deduplicate same path', function () {
70+
const errors: t.Errors = [
71+
{ value: {}, context: [{ key: 'value', type: t.string }] },
72+
{ value: {}, context: [{ key: 'value', type: t.number }] },
73+
];
74+
assert.strictEqual((formatValidationErrors(errors).match(/'value'/g) || []).length, 1);
75+
});
76+
});

modules/express/test/unit/typedRoutes/generateWallet.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,8 @@ describe('Generate Wallet Typed Routes Tests', function () {
405405
});
406406

407407
res.status.should.equal(400);
408-
res.body.should.be.an.Array();
409-
res.body[0].should.match(/label/);
408+
res.body.should.have.property('error');
409+
res.body.error.should.match(/label/);
410410
});
411411

412412
it('should return 400 when label is not a string', async function () {
@@ -418,8 +418,8 @@ describe('Generate Wallet Typed Routes Tests', function () {
418418
});
419419

420420
res.status.should.equal(400);
421-
res.body.should.be.an.Array();
422-
res.body[0].should.match(/label/);
421+
res.body.should.have.property('error');
422+
res.body.error.should.match(/label/);
423423
});
424424

425425
it('should return 400 when type is invalid', async function () {
@@ -432,8 +432,8 @@ describe('Generate Wallet Typed Routes Tests', function () {
432432
});
433433

434434
res.status.should.equal(400);
435-
res.body.should.be.an.Array();
436-
res.body[0].should.match(/type/);
435+
res.body.should.have.property('error');
436+
res.body.error.should.match(/type/);
437437
});
438438

439439
it('should return 400 when multisigType is invalid', async function () {
@@ -446,8 +446,8 @@ describe('Generate Wallet Typed Routes Tests', function () {
446446
});
447447

448448
res.status.should.equal(400);
449-
res.body.should.be.an.Array();
450-
res.body[0].should.match(/multisigType/);
449+
res.body.should.have.property('error');
450+
res.body.error.should.match(/multisigType/);
451451
});
452452

453453
it('should return 400 when backupXpubProvider is invalid', async function () {
@@ -460,8 +460,8 @@ describe('Generate Wallet Typed Routes Tests', function () {
460460
});
461461

462462
res.status.should.equal(400);
463-
res.body.should.be.an.Array();
464-
res.body[0].should.match(/backupXpubProvider/);
463+
res.body.should.have.property('error');
464+
res.body.error.should.match(/backupXpubProvider/);
465465
});
466466

467467
it('should return 400 when disableTransactionNotifications is not boolean', async function () {
@@ -474,8 +474,8 @@ describe('Generate Wallet Typed Routes Tests', function () {
474474
});
475475

476476
res.status.should.equal(400);
477-
res.body.should.be.an.Array();
478-
res.body[0].should.match(/disableTransactionNotifications/);
477+
res.body.should.have.property('error');
478+
res.body.error.should.match(/disableTransactionNotifications/);
479479
});
480480
});
481481

modules/express/test/unit/typedRoutes/isWalletAddress.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,7 @@ describe('IsWalletAddress codec tests', function () {
945945
.send(requestBody);
946946

947947
assert.strictEqual(result.status, 400);
948-
assert.ok(Array.isArray(result.body));
948+
assert.ok(result.body.error);
949949
});
950950

951951
it('should return 400 for missing keychains field', async function () {
@@ -960,7 +960,7 @@ describe('IsWalletAddress codec tests', function () {
960960
.send(requestBody);
961961

962962
assert.strictEqual(result.status, 400);
963-
assert.ok(Array.isArray(result.body));
963+
assert.ok(result.body.error);
964964
});
965965

966966
it('should return 400 for invalid keychains (not an array)', async function () {
@@ -976,7 +976,7 @@ describe('IsWalletAddress codec tests', function () {
976976
.send(requestBody);
977977

978978
assert.strictEqual(result.status, 400);
979-
assert.ok(Array.isArray(result.body));
979+
assert.ok(result.body.error);
980980
});
981981

982982
it('should return 400 for invalid walletVersion type', async function () {
@@ -993,7 +993,7 @@ describe('IsWalletAddress codec tests', function () {
993993
.send(requestBody);
994994

995995
assert.strictEqual(result.status, 400);
996-
assert.ok(Array.isArray(result.body));
996+
assert.ok(result.body.error);
997997
});
998998

999999
it('should return 400 for invalid derivedFromParentWithSeed type', async function () {
@@ -1010,7 +1010,7 @@ describe('IsWalletAddress codec tests', function () {
10101010
.send(requestBody);
10111011

10121012
assert.strictEqual(result.status, 400);
1013-
assert.ok(Array.isArray(result.body));
1013+
assert.ok(result.body.error);
10141014
});
10151015

10161016
it('should handle isWalletAddress throwing InvalidAddressError', async function () {

modules/express/test/unit/typedRoutes/lightningPayment.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -691,10 +691,10 @@ describe('Lightning Payment API Tests', function () {
691691
.send(requestBody);
692692

693693
assert.strictEqual(result.status, 400);
694-
result.body.should.be.Array();
695-
result.body.length.should.be.above(0);
694+
result.body.should.have.property('error');
695+
result.body.error.length.should.be.above(0);
696696
// Validation error should mention missing invoice field
697-
result.body[0].should.match(/invoice/);
697+
result.body.error.should.match(/invoice/);
698698
});
699699

700700
it('should return 400 when passphrase is missing', async function () {
@@ -709,10 +709,10 @@ describe('Lightning Payment API Tests', function () {
709709
.send(requestBody);
710710

711711
assert.strictEqual(result.status, 400);
712-
result.body.should.be.Array();
713-
result.body.length.should.be.above(0);
712+
result.body.should.have.property('error');
713+
result.body.error.length.should.be.above(0);
714714
// Validation error should mention missing passphrase field
715-
result.body[0].should.match(/passphrase/);
715+
result.body.error.should.match(/passphrase/);
716716
});
717717

718718
it('should return 400 when amountMsat is invalid format', async function () {
@@ -729,10 +729,10 @@ describe('Lightning Payment API Tests', function () {
729729
.send(requestBody);
730730

731731
assert.strictEqual(result.status, 400);
732-
result.body.should.be.Array();
733-
result.body.length.should.be.above(0);
732+
result.body.should.have.property('error');
733+
result.body.error.length.should.be.above(0);
734734
// Validation error should mention amountMsat field
735-
result.body[0].should.match(/amountMsat/);
735+
result.body.error.should.match(/amountMsat/);
736736
});
737737

738738
it('should handle wallet not found error', async function () {

0 commit comments

Comments
 (0)