From 54f762c845912b45811f6481518a100f10c5e1e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 13 Aug 2020 14:52:17 +0200 Subject: [PATCH] feat(rest): improve validation errors for invalid parameter value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework the validation code to use exiting `RestHttpErrors.invalidData` error instead of a custom `BadRequest` error. This way the error object includes the parameter name in the error message & properties and has a machine-readable `code` property. Signed-off-by: Miroslav Bajtoš --- .../coercion/coercion.acceptance.ts | 29 +++++++++++++++ .../rest/src/coercion/coerce-parameter.ts | 2 +- packages/rest/src/rest-http-error.ts | 8 +++-- packages/rest/src/types.ts | 5 +++ .../src/validation/request-body.validator.ts | 35 ++++++++++--------- 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/packages/rest/src/__tests__/acceptance/coercion/coercion.acceptance.ts b/packages/rest/src/__tests__/acceptance/coercion/coercion.acceptance.ts index 294d16d36b09..fe4a703e81ec 100644 --- a/packages/rest/src/__tests__/acceptance/coercion/coercion.acceptance.ts +++ b/packages/rest/src/__tests__/acceptance/coercion/coercion.acceptance.ts @@ -15,6 +15,7 @@ import { import { Client, createRestAppClient, + expect, givenHttpServerConfig, sinon, } from '@loopback/testlab'; @@ -253,6 +254,34 @@ describe('Coercion', () => { sinon.assert.calledWithExactly(spy, {...inclusionFilter}); }); + it('returns AJV validation errors in error details', async () => { + const filter = { + where: 'string-instead-of-object', + }; + const response = await client + .get(`/nested-inclusion-from-query`) + .query({filter: JSON.stringify(filter)}) + .expect(400); + + expect(response.body.error).to.containDeep({ + code: 'INVALID_PARAMETER_VALUE', + details: [ + { + code: 'type', + info: { + type: 'object', + }, + message: 'should be object', + path: '/where', + }, + ], + }); + + expect(response.body.error.message).to.match( + /Invalid data.* for parameter "filter"/, + ); + }); + async function givenAClient() { app = new RestApplication({rest: givenHttpServerConfig()}); app.controller(MyController); diff --git a/packages/rest/src/coercion/coerce-parameter.ts b/packages/rest/src/coercion/coerce-parameter.ts index d93d6aeffa00..d88ac9d16bc6 100644 --- a/packages/rest/src/coercion/coerce-parameter.ts +++ b/packages/rest/src/coercion/coerce-parameter.ts @@ -187,7 +187,7 @@ async function coerceObject( data, schema, {}, - {...options, coerceTypes: true, source: 'parameter'}, + {...options, coerceTypes: true, source: 'parameter', name: spec.name}, ); } diff --git a/packages/rest/src/rest-http-error.ts b/packages/rest/src/rest-http-error.ts index 259b10e50783..76644fa8566f 100644 --- a/packages/rest/src/rest-http-error.ts +++ b/packages/rest/src/rest-http-error.ts @@ -11,7 +11,7 @@ export namespace RestHttpErrors { name: string, extraProperties?: Props, ): HttpErrors.HttpError & Props { - const msg = `Invalid data ${JSON.stringify(data)} for parameter ${name}!`; + const msg = `Invalid data ${JSON.stringify(data)} for parameter "${name}".`; return Object.assign( new HttpErrors.BadRequest(msg), { @@ -51,11 +51,15 @@ export namespace RestHttpErrors { export const INVALID_REQUEST_BODY_MESSAGE = 'The request body is invalid. See error object `details` property for more info.'; - export function invalidRequestBody(): HttpErrors.HttpError { + + export function invalidRequestBody( + details: ValidationErrorDetails[], + ): HttpErrors.HttpError & {details: ValidationErrorDetails[]} { return Object.assign( new HttpErrors.UnprocessableEntity(INVALID_REQUEST_BODY_MESSAGE), { code: 'VALIDATION_FAILED', + details, }, ); } diff --git a/packages/rest/src/types.ts b/packages/rest/src/types.ts index 253f386cf4cd..557b00cc2bd7 100644 --- a/packages/rest/src/types.ts +++ b/packages/rest/src/types.ts @@ -117,6 +117,11 @@ export interface ValueValidationOptions extends ValidationOptions { * 'query', 'cookie', etc... */ source?: string; + + /** + * Parameter name, as provided in `ParameterObject#name` property. + */ + name?: string; } /** diff --git a/packages/rest/src/validation/request-body.validator.ts b/packages/rest/src/validation/request-body.validator.ts index ead149e5758f..556ffbf976b0 100644 --- a/packages/rest/src/validation/request-body.validator.ts +++ b/packages/rest/src/validation/request-body.validator.ts @@ -12,7 +12,6 @@ import { } from '@loopback/openapi-v3'; import ajv, {Ajv} from 'ajv'; import debugModule from 'debug'; -import _ from 'lodash'; import util from 'util'; import {HttpErrors, RequestBody, RestHttpErrors} from '..'; import { @@ -180,30 +179,32 @@ export async function validateValueAgainstSchema( // Throw invalid request body error if (options.source === 'body') { - const error = RestHttpErrors.invalidRequestBody(); - addErrorDetails(error, validationErrors); + const error = RestHttpErrors.invalidRequestBody( + buildErrorDetails(validationErrors), + ); throw error; } // Throw invalid value error - const error = new HttpErrors.BadRequest('Invalid value.'); - addErrorDetails(error, validationErrors); + const error = RestHttpErrors.invalidData(value, options.name ?? '(unknown)', { + details: buildErrorDetails(validationErrors), + }); throw error; } -function addErrorDetails( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - error: any, +function buildErrorDetails( validationErrors: ajv.ErrorObject[], -) { - error.details = _.map(validationErrors, e => { - return { - path: e.dataPath, - code: e.keyword, - message: e.message, - info: e.params, - }; - }); +): RestHttpErrors.ValidationErrorDetails[] { + return validationErrors.map( + (e: ajv.ErrorObject): RestHttpErrors.ValidationErrorDetails => { + return { + path: e.dataPath, + code: e.keyword, + message: e.message ?? `must pass validation rule ${e.keyword}`, + info: e.params, + }; + }, + ); } /**