From 86065fa2083d70ab18f0c64fb13b1120f4f0a474 Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Wed, 2 Mar 2022 12:50:15 +0800 Subject: [PATCH] feat: error handle middleware (#214) * feat: error handle middleware * feat: application error handler * feat: handle with sequelizeValidationError * fix: test * fix: test --- jest.setup.ts | 7 --- .../__tests__/create-with-expection.test.ts | 41 ++++++++++++++++ packages/plugin-users/src/server.ts | 2 - .../server/src/__tests__/error-handle.test.ts | 48 +++++++++++++++++++ packages/server/src/application.ts | 5 ++ packages/server/src/error-handler.ts | 44 +++++++++++++++++ packages/server/src/helper.ts | 18 +++++++ 7 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 packages/actions/src/__tests__/create-with-expection.test.ts create mode 100644 packages/server/src/__tests__/error-handle.test.ts create mode 100644 packages/server/src/error-handler.ts diff --git a/jest.setup.ts b/jest.setup.ts index 5b3e4dfa0..2c95f5081 100644 --- a/jest.setup.ts +++ b/jest.setup.ts @@ -7,14 +7,7 @@ jest.setTimeout(300000); // 把 console.error 转换成 error，方便断言 (() => { const spy = jest.spyOn(console, 'error'); - beforeAll(() => { - spy.mockImplementation((message) => { - console.log(message); - throw new Error(message); - }); - }); afterAll(() => { spy.mockRestore(); }); })(); - diff --git a/packages/actions/src/__tests__/create-with-expection.test.ts b/packages/actions/src/__tests__/create-with-expection.test.ts new file mode 100644 index 000000000..77473bacd --- /dev/null +++ b/packages/actions/src/__tests__/create-with-expection.test.ts @@ -0,0 +1,41 @@ +import { mockServer } from '@nocobase/test'; + +describe('create with exception', () => { + it('should handle validationError', async () => { + const app = mockServer(); + + app.collection({ + name: 'users', + fields: [ + { + name: 'name', + type: 'string', + allowNull: false, + }, + ], + }); + + await app.loadAndInstall(); + + const response = await app + .agent() + .resource('users') + .create({ + values: { + title: 't1', + }, + }); + + expect(response.statusCode).toEqual(400); + + expect(response.body).toEqual({ + errors: [ + { + message: 'users.name cannot be null', + }, + ], + }); + + await app.destroy(); + }); +}); diff --git a/packages/plugin-users/src/server.ts b/packages/plugin-users/src/server.ts index d1cc9191f..92ce973a7 100644 --- a/packages/plugin-users/src/server.ts +++ b/packages/plugin-users/src/server.ts @@ -5,7 +5,6 @@ import * as actions from './actions/users'; import * as middlewares from './middlewares'; export default class UsersPlugin extends Plugin { - async beforeLoad() { this.db.on('users.afterCreateWithAssociations', async (model, options) => { const { transaction } = options; @@ -85,5 +84,4 @@ export default class UsersPlugin extends Plugin { }, }); } - } diff --git a/packages/server/src/__tests__/error-handle.test.ts b/packages/server/src/__tests__/error-handle.test.ts new file mode 100644 index 000000000..0b993f600 --- /dev/null +++ b/packages/server/src/__tests__/error-handle.test.ts @@ -0,0 +1,48 @@ +import { mockServer } from '@nocobase/test'; + +describe('error handle', () => { + it('should handle error with default handler', async () => { + const app = mockServer(); + + app.use(async () => { + throw new Error('some thing went wrong'); + }); + + const response = await app.agent().post('/'); + + expect(response.statusCode).toEqual(500); + expect(response.body.errors[0].message).toEqual('some thing went wrong'); + }); + + it('should handle error by custom handler', async () => { + class CustomError extends Error { + constructor(message, errors) { + super(message); + this.name = 'CustomError'; + } + } + + const app = mockServer(); + + app.errorHandler.register( + (err) => { + return err.name == 'CustomError'; + }, + (err, ctx) => { + ctx.body = { + message: 'hello', + }; + ctx.status = 422; + }, + ); + + app.use(async () => { + throw new CustomError('some thing went wrong', []); + }); + + const response = await app.agent().post('/'); + + expect(response.statusCode).toEqual(422); + expect(response.body).toEqual({ message: 'hello' }); + }); +}); diff --git a/packages/server/src/application.ts b/packages/server/src/application.ts index 0ce33811b..da017414f 100644 --- a/packages/server/src/application.ts +++ b/packages/server/src/application.ts @@ -12,6 +12,7 @@ import { createACL } from './acl'; import { createCli, createDatabase, createI18n, createResourcer, registerMiddlewares } from './helper'; import { Plugin } from './plugin'; import { PluginManager, InstallOptions } from './plugin-manager'; +import { ErrorHandler } from './error-handler'; export interface ResourcerOptions { prefix?: string; @@ -84,6 +85,8 @@ export class Application exten public readonly acl: ACL; + public readonly errorHandler: ErrorHandler; + protected plugins = new Map(); public listenServer: Server; @@ -101,6 +104,8 @@ export class Application exten app: this, }); + this.errorHandler = new ErrorHandler(this); + registerMiddlewares(this, options); if (options.registerActions !== false) { registerActions(this); diff --git a/packages/server/src/error-handler.ts b/packages/server/src/error-handler.ts new file mode 100644 index 000000000..5d58a38cd --- /dev/null +++ b/packages/server/src/error-handler.ts @@ -0,0 +1,44 @@ +import Application from './application'; + +export class ErrorHandler { + handlers = []; + + constructor(app: Application) {} + + register(guard: (err) => boolean, render: (err, ctx) => void) { + this.handlers.push({ + guard, + render, + }); + } + + defaultHandler(err, ctx) { + ctx.status = err.statusCode || err.status || 500; + ctx.body = { + errors: [ + { + message: err.message, + code: err.code, + }, + ], + }; + } + + middleware() { + const self = this; + + return async function errorHandler(ctx, next) { + try { + await next(); + } catch (err) { + for (const handler of self.handlers) { + if (handler.guard(err)) { + return handler.render(err, ctx); + } + } + + self.defaultHandler(err, ctx); + } + }; + } +} diff --git a/packages/server/src/helper.ts b/packages/server/src/helper.ts index ce7e951c5..d69d498dc 100644 --- a/packages/server/src/helper.ts +++ b/packages/server/src/helper.ts @@ -9,6 +9,8 @@ import Application, { ApplicationOptions } from './application'; import { dataWrapping } from './middlewares/data-wrapping'; import { table2resource } from './middlewares/table2resource'; +import ValidationError from 'sequelize'; + export function createDatabase(options: ApplicationOptions) { if (options.database instanceof Database) { return options.database; @@ -93,7 +95,23 @@ export function createCli(app: Application, options: ApplicationOptions): Comman return cli; } +function registerErrorHandler(app: Application) { + app.errorHandler.register( + (err) => err.name == 'SequelizeValidationError', + (err, ctx) => { + ctx.body = { + errors: err.errors.map((err) => ({ message: err.message })), + }; + + ctx.status = 400; + }, + ); + app.use(app.errorHandler.middleware()); +} + export function registerMiddlewares(app: Application, options: ApplicationOptions) { + registerErrorHandler(app); + if (options.bodyParser !== false) { app.use( bodyParser({