diff --git a/README.md b/README.md index e20c574..d6bf0be 100644 --- a/README.md +++ b/README.md @@ -828,7 +828,6 @@ you now have full CRUD for your new todos service! and to [seed](http://knexjs.org/#Seeds) a table with mock data. ## Error handling - As of version 4.8.0, `feathers-objection` only throws [Feathers Errors](https://docs.feathersjs.com/api/errors.html) with the message. On the server, the original error can be retrieved through a secure symbol via `error[require('feathers-objection').ERROR]`. @@ -845,6 +844,10 @@ try { } ``` +As of version 7.0.0, `feathers-objection` has normalized errors accross all databases supported by Objection, and makes a best-effort attempt to provide reasonable error messages that can be returned directly to the client. + +If these error messages do not work for your needs, the original error is still available using the symbol described above. + ## Migrating to `feathers-objection` v2 `feathers-objection` 2.0.0 comes with important security and usability updates @@ -887,6 +890,17 @@ The following breaking changes have been introduced: - `patch` method now enforce `params.query` with upsert - NotFound error will be thrown when `get` & `update` methods are called with different values in `id` & `params.query.id` +## Migrating to `feathers-objection` v7 + +`feathers-objection` 7.0.0 comes with improved error handling. + +The following breaking changes have been introduced: + +- All Databases will return the same types of errors based on the underlying Objection error +- SQL Driver error text is no longer used as the Feathers error message +- Objection errors are mapped more accurately to Feathers errors, e.g. + - Objection's `UniqueViolationError` -> Feathers' `Conflict` error type + ## License Copyright © 2020 diff --git a/src/error-handler.js b/src/error-handler.js index 13d1ca1..0fe918c 100644 --- a/src/error-handler.js +++ b/src/error-handler.js @@ -1,108 +1,75 @@ import errors from '@feathersjs/errors'; +import { + ValidationError, + NotFoundError, + DBError, + ConstraintViolationError, + UniqueViolationError, + NotNullViolationError, + ForeignKeyViolationError, + CheckViolationError, + DataError +} from 'objection'; const ERROR = Symbol('feathers-knex/error'); export default function errorHandler (error) { const { message } = error.nativeError || error; - let feathersError = error; + let feathersError; - if (error.code === 'SQLITE_ERROR') { - switch (error.errno) { - case 1: - case 8: - case 18: - case 19: - case 20: - feathersError = new errors.BadRequest(message); + if (error instanceof errors.FeathersError) { + feathersError = error; + } else if (error instanceof ValidationError) { + switch (error.type) { + case 'ModelValidation': + feathersError = new errors.BadRequest(message, error.data); break; - case 2: - feathersError = new errors.Unavailable(message); + case 'RelationExpression': + feathersError = new errors.BadRequest('Invalid Relation Expression'); break; - case 3: - case 23: - feathersError = new errors.Forbidden(message); + case 'UnallowedRelation': + feathersError = new errors.BadRequest('Unallowed Relation Expression'); break; - case 12: - feathersError = new errors.NotFound(message); + case 'InvalidGraph': + feathersError = new errors.BadRequest('Invalid Relation Graph'); break; default: - feathersError = new errors.GeneralError(message); - break; - } - } else if (error.statusCode) { // Objection validation error - switch (error.statusCode) { - case 400: - feathersError = new errors.BadRequest(message); - break; - - case 401: - feathersError = new errors.NotAuthenticated(message); - break; - - case 402: - feathersError = new errors.PaymentError(message); - break; - - case 403: - feathersError = new errors.Forbidden(message); - break; - - case 404: - feathersError = new errors.NotFound(message); - break; - - case 405: - feathersError = new errors.MethodNotAllowed(message); - break; - - case 406: - feathersError = new errors.NotAcceptable(message); - break; - - case 408: - feathersError = new errors.Timeout(message); - break; - - case 409: - feathersError = new errors.Conflict(message); - break; - - case 422: - feathersError = new errors.Unprocessable(message); - break; - - case 501: - feathersError = new errors.NotImplemented(message); - break; - - case 503: - feathersError = new errors.Unavailable(message); - break; - - case 500: - default: - feathersError = new errors.GeneralError(message); - } - } else if (typeof error.code === 'string') { // Postgres error code - TODO: Properly detect postgres error - const pgerror = error.code.substring(0, 2); - - switch (pgerror) { - case '28': - case '42': - feathersError = new errors.Forbidden(message); - break; - - case '20': - case '21': - case '22': - case '23': - feathersError = new errors.BadRequest(message); - break; - - default: - feathersError = new errors.GeneralError(message); + feathersError = new errors.BadRequest('Unknown Validation Error'); } - } else if (!(error instanceof errors.FeathersError)) { + } else if (error instanceof NotFoundError) { + feathersError = new errors.NotFound(message); + } else if (error instanceof UniqueViolationError) { + feathersError = new errors.Conflict(`${error.columns.join(', ')} must be unique`, { + columns: error.columns, + table: error.table, + constraint: error.constraint + }); + } else if (error instanceof NotNullViolationError) { + feathersError = new errors.BadRequest(`${error.column} must not be null`, { + column: error.column, + table: error.table + }); + } else if (error instanceof ForeignKeyViolationError) { + feathersError = new errors.Conflict('Foreign Key Violation', { + table: error.table, + constraint: error.constraint + }); + } else if (error instanceof CheckViolationError) { + feathersError = new errors.BadRequest('Check Violation', { + table: error.table, + constraint: error.constraint + }); + } else if (error instanceof ConstraintViolationError) { + feathersError = new errors.Conflict('Constraint Violation', { + columns: error.columns, + table: error.table, + constraint: error.constraint + }); + } else if (error instanceof DataError) { + feathersError = new errors.BadRequest('Invalid Data'); + } else if (error instanceof DBError) { + feathersError = new errors.GeneralError('Unknown Database Error'); + } else { feathersError = new errors.GeneralError(message); } diff --git a/test/index.test.js b/test/index.test.js index a63df66..3b0f895 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -15,7 +15,18 @@ import PeopleRoomsCustomIdSeparator from './people-rooms-custom-id-separator'; import Company from './company'; import Employee from './employee'; import Client from './client'; -import { Model, UniqueViolationError } from 'objection'; +import { + CheckViolationError, + ConstraintViolationError, + DataError, + DBError, + ForeignKeyViolationError, + Model, + NotFoundError, + NotNullViolationError, + UniqueViolationError, + ValidationError +} from 'objection'; const testSuite = adapterTests([ '.options', @@ -325,205 +336,89 @@ describe('Feathers Objection Service', () => { } }); - describe('SQLite', () => { - it('Unknown error code', () => { + describe('Error Mappings', () => { + it('Unknown error', () => { const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 999; expect(errorHandler.bind(null, error)).to.throw(errors.GeneralError); }); - it('BadRequest 1', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 1; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); - - it('BadRequest 8', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 8; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); - - it('BadRequest 18', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 18; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); - - it('BadRequest 19', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 19; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); - - it('BadRequest 20', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 20; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); - - it('Unavailable 2', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 2; - expect(errorHandler.bind(null, error)).to.throw(errors.Unavailable); - }); - - it('Forbidden 3', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 3; - expect(errorHandler.bind(null, error)).to.throw(errors.Forbidden); - }); - - it('Forbidden 23', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 23; - expect(errorHandler.bind(null, error)).to.throw(errors.Forbidden); - }); - - it('NotFound 12', () => { - const error = new Error(); - error.code = 'SQLITE_ERROR'; - error.errno = 12; - expect(errorHandler.bind(null, error)).to.throw(errors.NotFound); - }); - }); - - describe('Objection', () => { - it('Unknown error code', () => { - const error = new Error(); - error.statusCode = 999; - expect(errorHandler.bind(null, error)).to.throw(errors.GeneralError); - }); - - it('BadRequest 400', () => { - const error = new Error(); - error.statusCode = 400; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); - - it('NotAuthenticated 401', () => { - const error = new Error(); - error.statusCode = 401; - expect(errorHandler.bind(null, error)).to.throw(errors.NotAuthenticated); - }); - - it('PaymentError 402', () => { - const error = new Error(); - error.statusCode = 402; - expect(errorHandler.bind(null, error)).to.throw(errors.PaymentError); - }); - - it('Forbidden 403', () => { - const error = new Error(); - error.statusCode = 403; - expect(errorHandler.bind(null, error)).to.throw(errors.Forbidden); + it('Validation error', () => { + const validationErrTypes = ['ModelValidation', 'RelationExpression', 'UnallowedRelation', 'InvalidGraph', 'unknown-thing']; + for (const type of validationErrTypes) { + const error = new ValidationError({ type }); + expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); + } }); - it('NotFound 404', () => { - const error = new Error(); - error.statusCode = 404; + it('NotFound error', () => { + const error = new NotFoundError(); expect(errorHandler.bind(null, error)).to.throw(errors.NotFound); }); - it('MethodNotAllowed 405', () => { - const error = new Error(); - error.statusCode = 405; - expect(errorHandler.bind(null, error)).to.throw(errors.MethodNotAllowed); - }); - - it('NotAcceptable 406', () => { - const error = new Error(); - error.statusCode = 406; - expect(errorHandler.bind(null, error)).to.throw(errors.NotAcceptable); - }); - - it('Timeout 408', () => { - const error = new Error(); - error.statusCode = 408; - expect(errorHandler.bind(null, error)).to.throw(errors.Timeout); - }); + it('UniqueViolation error', () => { + const error = new UniqueViolationError({ + nativeError: new Error(), + client: 'sqlite', + table: 'tableName', + columns: ['columnName'] + }); - it('Conflict 409', () => { - const error = new Error(); - error.statusCode = 409; expect(errorHandler.bind(null, error)).to.throw(errors.Conflict); }); - it('Unprocessable 422', () => { - const error = new Error(); - error.statusCode = 422; - expect(errorHandler.bind(null, error)).to.throw(errors.Unprocessable); - }); + it('ConstraintViolation error', () => { + const error = new ConstraintViolationError({ + nativeError: new Error(), + client: 'sqlite' + }); - it('GeneralError 500', () => { - const error = new Error(); - error.statusCode = 500; - expect(errorHandler.bind(null, error)).to.throw(errors.GeneralError); + expect(errorHandler.bind(null, error)).to.throw(errors.Conflict); }); - it('NotImplemented 501', () => { - const error = new Error(); - error.statusCode = 501; - expect(errorHandler.bind(null, error)).to.throw(errors.NotImplemented); - }); + it('NotNullViolation error', () => { + const error = new NotNullViolationError({ + nativeError: new Error(), + client: 'sqlite', + column: 'columnName' + }); - it('Unavailable 503', () => { - const error = new Error(); - error.statusCode = 503; - expect(errorHandler.bind(null, error)).to.throw(errors.Unavailable); + expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); }); - }); - describe('Postgres', () => { - it('Unknown error code', () => { - const error = new Error(); - error.code = '999'; - expect(errorHandler.bind(null, error)).to.throw(errors.GeneralError); - }); + it('ForeignKeyViolation error', () => { + const error = new ForeignKeyViolationError({ + nativeError: new Error(), + client: 'sqlite' + }); - it('Forbidden 28', () => { - const error = new Error(); - error.code = '28'; - expect(errorHandler.bind(null, error)).to.throw(errors.Forbidden); + expect(errorHandler.bind(null, error)).to.throw(errors.Conflict); }); - it('Forbidden 42', () => { - const error = new Error(); - error.code = '42'; - expect(errorHandler.bind(null, error)).to.throw(errors.Forbidden); - }); + it('CheckViolation error', () => { + const error = new CheckViolationError({ + nativeError: new Error(), + client: 'sqlite' + }); - it('BadRequest 20', () => { - const error = new Error(); - error.code = '20'; expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); }); - it('BadRequest 21', () => { - const error = new Error(); - error.code = '21'; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); - }); + it('Data error', () => { + const error = new DataError({ + nativeError: new Error(), + client: 'sqlite' + }); - it('BadRequest 22', () => { - const error = new Error(); - error.code = '22'; expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); }); - it('BadRequest 23', () => { - const error = new Error(); - error.code = '23'; - expect(errorHandler.bind(null, error)).to.throw(errors.BadRequest); + it('Database error', () => { + const error = new DBError({ + nativeError: new Error(), + client: 'sqlite' + }); + + expect(errorHandler.bind(null, error)).to.throw(errors.GeneralError); }); }); }); @@ -1548,7 +1443,7 @@ describe('Feathers Objection Service', () => { }).catch(function (error) { expect(error).to.be.ok; expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{numberField}\' AS text) = 1.5 - SQLITE_ERROR: unrecognized token: "#"'); + expect(error[ERROR].message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{numberField}\' AS text) = 1.5 - SQLITE_ERROR: unrecognized token: "#"'); }); }); @@ -1558,7 +1453,7 @@ describe('Feathers Objection Service', () => { }).catch(function (error) { expect(error).to.be.ok; expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{numberField}\' AS text) > 1.5 - SQLITE_ERROR: unrecognized token: "#"'); + expect(error[ERROR].message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{numberField}\' AS text) > 1.5 - SQLITE_ERROR: unrecognized token: "#"'); }); }); @@ -1568,7 +1463,7 @@ describe('Feathers Objection Service', () => { }).catch(function (error) { expect(error).to.be.ok; expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{objectField,object}\' AS text) = \'string in jsonObject.objectField.object\' - SQLITE_ERROR: unrecognized token: "#"'); + expect(error[ERROR].message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{objectField,object}\' AS text) = \'string in jsonObject.objectField.object\' - SQLITE_ERROR: unrecognized token: "#"'); }); }); @@ -1584,7 +1479,7 @@ describe('Feathers Objection Service', () => { }).catch(function (error) { expect(error).to.be.ok; expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonArray`#>>\'{0,objectField,object}\' AS text) = \'I\'\'m string in jsonArray[0].objectField.object\' - SQLITE_ERROR: unrecognized token: "#"'); + expect(error[ERROR].message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonArray`#>>\'{0,objectField,object}\' AS text) = \'I\'\'m string in jsonArray[0].objectField.object\' - SQLITE_ERROR: unrecognized token: "#"'); }); }); }); @@ -1888,8 +1783,8 @@ describe('Feathers Objection Service', () => { it('Rollback on sub insert failure', () => { // Dan Davis already exists return companies.create({ name: 'Compaq', clients: [{ name: 'Dan Davis' }] }, { atomic: true }).catch((error) => { - expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.match(/SQLITE_CONSTRAINT: UNIQUE/); + expect(error instanceof errors.Conflict).to.be.ok; + expect(error[ERROR].message).to.match(/SQLITE_CONSTRAINT: UNIQUE/); return companies.find({ query: { name: 'Compaq', $eager: 'clients' } }).then( (data) => { expect(data.length).to.equal(0); @@ -1900,8 +1795,8 @@ describe('Feathers Objection Service', () => { it('Rollback on multi insert failure', () => { // Google already exists return companies.create([{ name: 'Google' }, { name: 'Compaq' }], { atomic: true }).catch((error) => { - expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.match(/SQLITE_CONSTRAINT: UNIQUE/); + expect(error instanceof errors.Conflict).to.be.ok; + expect(error[ERROR].message).to.match(/SQLITE_CONSTRAINT: UNIQUE/); return companies.find({ query: { name: 'Compaq' } }).then( (data) => { expect(data.length).to.equal(0); @@ -1926,8 +1821,8 @@ describe('Feathers Objection Service', () => { } ] }, { atomic: true }).catch((error) => { - expect(error instanceof errors.GeneralError).to.be.ok; - expect(error.message).to.match(/SQLITE_CONSTRAINT: UNIQUE/); + expect(error instanceof errors.Conflict).to.be.ok; + expect(error[ERROR].message).to.match(/SQLITE_CONSTRAINT: UNIQUE/); return companies.find({ query: { name: 'Google', $eager: 'clients' } }).then( (data) => { expect(data.length).to.equal(1);