fix(schema): Properly handle resolver errors (#2540)

feathersjs · Feb 15, 2022 · 31fbdff · 31fbdff
1 parent b9dfaee
commit 31fbdff
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 4 deletions.
diff --git a/packages/schema/src/resolver.ts b/packages/schema/src/resolver.ts
@@ -1,5 +1,4 @@
 import { BadRequest } from '@feathersjs/errors';
-import { Schema } from './schema';
 
 export type PropertyResolver<T, V, C> = (
   value: V|undefined,
@@ -13,7 +12,9 @@ export type PropertyResolverMap<T, C> = {
 }
 
 export interface ResolverConfig<T, C> {
-  schema?: Schema<any>,
+  // TODO this should be `Schema<any>` but has recently produced an error, see
+  // https://github.com/ThomasAribart/json-schema-to-ts/issues/53
+  schema?: any,
   validate?: 'before'|'after'|false,
   properties: PropertyResolverMap<T, C>
 }
@@ -94,7 +95,9 @@ export class Resolver<T, C> {
     }));
 
     if (hasErrors) {
-      throw new BadRequest(`Error resolving data ${status?.properties.join('.')}`, errors);
+      const propertyName = status?.properties ? ` ${status.properties.join('.')}` : '';
+
+      throw new BadRequest('Error resolving data' + propertyName, errors);
     }
 
     return schema && validate === 'after'

diff --git a/packages/schema/test/fixture.ts b/packages/schema/test/fixture.ts
@@ -2,6 +2,7 @@ import {
   feathers, HookContext, Application as FeathersApplication
 } from '@feathersjs/feathers';
 import { memory, Service } from '@feathersjs/memory';
+import { GeneralError } from '@feathersjs/errors';
 
 import {
   schema, resolve, Infer, resolveResult,
@@ -86,6 +87,10 @@ export const messageResultResolver = resolve<MessageResult, HookContext<Applicat
     user: async (_value, message, context) => {
       const { userId } = message;
 
+      if (context.params.error === true) {
+        throw new GeneralError('This is an error');
+      }
+
       return context.app.service('users').get(userId, context.params);
     }
   }

diff --git a/packages/schema/test/hooks.test.ts b/packages/schema/test/hooks.test.ts
@@ -24,7 +24,7 @@ describe('@feathersjs/schema/hooks', () => {
     });
   });
 
-  it('resolves results', async () => {
+  it('resolves results and handles resolver errors (#2534)', async () => {
     // eslint-disable-next-line
     const { password, ...externalUser } = user;
     const payload = {
@@ -49,6 +49,24 @@ describe('@feathersjs/schema/hooks', () => {
       user: externalUser,
       ...payload
     }]);
+
+    await assert.rejects(() => app.service('messages').find({
+      provider: 'external',
+      error: true
+    }), {
+      name: 'BadRequest',
+      message: 'Error resolving data',
+      code: 400,
+      className: 'bad-request',
+      data: {
+        user: {
+          name: 'GeneralError',
+          message: 'This is an error',
+          code: 500,
+          className: 'general-error'
+        }
+      }
+    });
   });
 
   it('validates and converts the query', async () => {