From 7e0976cd8cc65f012ee86f962aad08ce3aa16cb9 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Mon, 28 Oct 2019 10:33:47 -0400 Subject: [PATCH] When serviceData fails validation, include the service class in the stack trace (#4266) This should make it easier to debug #3784. --- core/base-service/base.js | 8 +++++- core/base-service/base.spec.js | 46 +++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/core/base-service/base.js b/core/base-service/base.js index 7422726128..352314d836 100644 --- a/core/base-service/base.js +++ b/core/base-service/base.js @@ -267,6 +267,12 @@ class BaseService { throw new Error(`Handler not implemented for ${this.constructor.name}`) } + // Making this an instance method ensures debuggability. + // https://github.com/badges/shields/issues/3784 + _validateServiceData(serviceData) { + Joi.assert(serviceData, serviceDataSchema) + } + _handleError(error) { if (error instanceof NotFound || error instanceof InvalidParameter) { trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error) @@ -379,7 +385,7 @@ class BaseService { namedParams, transformedQueryParams ) - Joi.assert(serviceData, serviceDataSchema) + serviceInstance._validateServiceData(serviceData) } catch (error) { serviceError = error } diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js index c1619983bf..9acdd90a48 100644 --- a/core/base-service/base.spec.js +++ b/core/base-service/base.spec.js @@ -192,7 +192,7 @@ describe('BaseService', function() { }) }) - it('Throws a validation error on invalid data', async function() { + context('On invalid data', function() { class ThrowingService extends DummyService { async handle() { return { @@ -200,19 +200,37 @@ describe('BaseService', function() { } } } - try { - await ThrowingService.invoke( - {}, - { handleInternalErrors: false }, - { namedParamA: 'bar.bar.bar' } - ) - expect.fail('Expected to throw') - } catch (e) { - expect(e.name).to.equal('ValidationError') - expect(e.details.map(({ message }) => message)).to.deep.equal([ - '"message" is required', - ]) - } + + it('Throws a validation error on invalid data', async function() { + try { + await ThrowingService.invoke( + {}, + { handleInternalErrors: false }, + { namedParamA: 'bar.bar.bar' } + ) + expect.fail('Expected to throw') + } catch (e) { + expect(e.name).to.equal('ValidationError') + expect(e.details.map(({ message }) => message)).to.deep.equal([ + '"message" is required', + ]) + } + }) + + // Ensure debuggabillity. + // https://github.com/badges/shields/issues/3784 + it('Includes the service class in the stack trace', async function() { + try { + await ThrowingService.invoke( + {}, + { handleInternalErrors: false }, + { namedParamA: 'bar.bar.bar' } + ) + expect.fail('Expected to throw') + } catch (e) { + expect(e.stack).to.include('ThrowingService._validateServiceData') + } + }) }) })