Skip to content

Commit

Permalink
Provide friendlier error messages for [endpoint] (#2858)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulmelnikow committed Jan 23, 2019
1 parent a8bedce commit 4e9763b
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 10 deletions.
13 changes: 11 additions & 2 deletions core/base-service/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,20 @@ class BaseService {
)
}

static _validate(data, schema, { allowAndStripUnknownKeys = true } = {}) {
static _validate(
data,
schema,
{
prettyErrorMessage = 'invalid response data',
includeKeys = false,
allowAndStripUnknownKeys = true,
} = {}
) {
return validate(
{
ErrorClass: InvalidResponse,
prettyErrorMessage: 'invalid response data',
prettyErrorMessage,
includeKeys,
traceErrorMessage: 'Response did not match schema',
traceSuccessMessage: 'Response after validation',
allowAndStripUnknownKeys,
Expand Down
2 changes: 1 addition & 1 deletion core/base-service/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function validate(
if (includeKeys) {
const keys = error.details.map(({ path }) => path)
if (keys) {
prettyMessage = `${prettyErrorMessage}: ${keys.join(',')}`
prettyMessage = `${prettyErrorMessage}: ${keys.join(', ')}`
}
}

Expand Down
19 changes: 14 additions & 5 deletions core/base-service/validate.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ describe('validate', function() {
try {
validate(
{ ...options, includeKeys: true },
{ requiredString: ['this', "shouldn't", 'work'] },
{
requiredString: ['this', "shouldn't", 'work'],
},
schema
)
expect.fail('Expected to throw')
Expand All @@ -107,12 +109,19 @@ describe('validate', function() {
})

it('allowAndStripUnknownKeys', function() {
expect(() =>
try {
validate(
{ ...options, allowAndStripUnknownKeys: false },
{ requiredString: 'bar', extra: 'nonsense' },
{ ...options, allowAndStripUnknownKeys: false, includeKeys: true },
{ requiredString: 'bar', extra: 'nonsense', more: 'bogus' },
schema
)
).to.throw(InvalidParameter, '"extra" is not allowed')
expect.fail('Expected to throw')
} catch (e) {
expect(e).to.be.an.instanceof(InvalidParameter)
expect(e.message).to.equal(
'Invalid Parameter: "extra" is not allowed. "more" is not allowed'
)
expect(e.prettyMessage).to.equal(`${prettyErrorMessage}: extra, more`)
}
})
})
2 changes: 2 additions & 0 deletions services/endpoint/endpoint.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ module.exports = class Endpoint extends BaseJsonService {
})
// Override the validation options because we want to reject unknown keys.
const validated = this.constructor._validate(json, endpointSchema, {
prettyErrorMessage: 'invalid properties',
includeKeys: true,
allowAndStripUnknownKeys: false,
})

Expand Down
10 changes: 8 additions & 2 deletions services/endpoint/endpoint.tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ t.create('Invalid schema (mocked)')
schemaVersion: -1,
})
)
.expectJSON({ name: 'custom badge', value: 'invalid response data' })
.expectJSON({
name: 'custom badge',
value: 'invalid properties: schemaVersion',
})

t.create('Invalid schema (mocked)')
.get('.json?url=https://example.com/badge')
Expand All @@ -161,7 +164,10 @@ t.create('Invalid schema (mocked)')
bogus: true,
})
)
.expectJSON({ name: 'custom badge', value: 'invalid response data' })
.expectJSON({
name: 'custom badge',
value: 'invalid properties: extra, bogus',
})

t.create('User color overrides success color')
.get('.json?url=https://example.com/badge&colorB=101010&style=_shields_test')
Expand Down

0 comments on commit 4e9763b

Please sign in to comment.