Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling is not working correctly for Postgres #130

Closed
GVanderLugt opened this issue Nov 19, 2020 · 4 comments · Fixed by #131
Closed

Error handling is not working correctly for Postgres #130

GVanderLugt opened this issue Nov 19, 2020 · 4 comments · Fixed by #131

Comments

@GVanderLugt
Copy link

Issue

My user model requires emails to be unique. If I try to create a user with a duplicate email address I get a 500 error with the DB error as the message.

GeneralError: insert into "users" ("created_on", "email", "isVerified", "name", "password", "updated_on", "verifyChanges", "verifyExpires", "verifyShortToken", "verifyToken") values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) returning "id" - duplicate key value violates unique constraint "users_email_unique"

Debugging

I've been trying to debug in error-handler.js. It looks like this error just isn't being handled. The incoming error object has all the necessary info to return a well formed Feathers error, but it's not doing anything.

Code

I have a very simple user service that looks like this:

users.service.ts

// Initializes the `user` service on path `/user`
import { ServiceAddons } from '@feathersjs/feathers';
import { Application } from '../../declarations';
import Users from './users.class';
import UserModel, { UserType } from '../../models/user.model';
import hooks from './users.hooks';

// Add this service to the service type index
declare module '../../declarations' {
  interface ServiceTypes {
    users: Users & ServiceAddons<UserType>;
  }
}

export default function (app: Application) {
  const options = {
    Model: UserModel,
    paginate: app.get('paginate'),
  };

  // Initialize our service with any options it requires
  app.use('/users', new Users(options, app));

  // Get our initialized service so that we can register hooks
  const service = app.service('users');

  service.hooks(hooks);
}

users.class.ts

import UserModel, { UserType } from '../../models/user.model';
import Base from '../base.class';

export default class Users extends Base<UserType, UserModel> {}

base.class is very simple and is simply extending feathers-objection.Service with some extra typing.

Dependencies

{
    "feathers-objection": "^6.1.1",
    "objection": "^2.2.3",
    "pg": "^8.4.1",
    "knex": "^0.21.6",
}
@GVanderLugt
Copy link
Author

GVanderLugt commented Nov 19, 2020

When I log the incoming error in error-handler.js it looks like it's an instance of an Objection UniqueViolationError and it's just not being handled correctly.

UniqueViolationError: insert into "users" ("created_on", "email", "isVerified", "name", "password", "updated_on", "verifyChanges", "verifyExpires", "verifyShortToken", "verifyToken") values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) returning "id" - duplicate key value violates unique constraint "users_email_unique"
app_1       |     at wrapError (/usr/src/app/node_modules/db-errors/lib/dbErrors.js:19:14)
app_1       |     at handleExecuteError (/usr/src/app/node_modules/objection/lib/queryBuilder/QueryBuilder.js:1506:32)
app_1       |     at QueryBuilder.execute (/usr/src/app/node_modules/objection/lib/queryBuilder/QueryBuilder.js:687:20)
app_1       |     at processTicksAndRejections (internal/process/task_queues.js:97:5) {
app_1       |   nativeError: error: insert into "users" ("created_on", "email", "isVerified", "name", "password", "updated_on", "verifyChanges", "verifyExpires", "verifyShortToken", "verifyToken") values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) returning "id" - duplicate key value violates unique constraint "users_email_unique"
app_1       |       at Function.value (/usr/src/app/node_modules/knex/lib/util/make-knex.js:90:29)
app_1       |       at buildKnexQuery (/usr/src/app/node_modules/objection/lib/queryBuilder/QueryBuilder.js:1591:63)
app_1       |       at doExecute (/usr/src/app/node_modules/objection/lib/queryBuilder/QueryBuilder.js:1482:31)
app_1       |       at QueryBuilder.execute (/usr/src/app/node_modules/objection/lib/queryBuilder/QueryBuilder.js:684:28) {
app_1       |     length: 227,
app_1       |     severity: 'ERROR',
app_1       |     code: '23505',
app_1       |     detail: 'Key (email)=(gerrit.vanderlugt+1@gmail.com) already exists.',
app_1       |     hint: undefined,
app_1       |     position: undefined,
app_1       |     internalPosition: undefined,
app_1       |     internalQuery: undefined,
app_1       |     where: undefined,
app_1       |     schema: 'public',
app_1       |     table: 'users',
app_1       |     column: undefined,
app_1       |     dataType: undefined,
app_1       |     constraint: 'users_email_unique',
app_1       |     file: 'nbtinsert.c',
app_1       |     line: '570',
app_1       |     routine: '_bt_check_unique',
app_1       |     originalStack: 'error: insert into "users" ("created_on", "email", "isVerified", "name", "password", "updated_on", "verifyChanges", "verifyExpires", "verifyShortToken", "verifyToken") values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) returning "id" - duplicate key value violates unique constraint "users_email_unique"\n' +
app_1       |       '    at Parser.parseErrorMessage (/usr/src/app/node_modules/pg-protocol/src/parser.ts:357:11)\n' +
app_1       |       '    at Parser.handlePacket (/usr/src/app/node_modules/pg-protocol/src/parser.ts:186:21)\n' +
app_1       |       '    at Parser.parse (/usr/src/app/node_modules/pg-protocol/src/parser.ts:101:30)\n' +
app_1       |       '    at Socket.<anonymous> (/usr/src/app/node_modules/pg-protocol/src/index.ts:7:48)\n' +
app_1       |       '    at Socket.emit (events.js:315:20)\n' +
app_1       |       '    at addChunk (_stream_readable.js:297:12)\n' +
app_1       |       '    at readableAddChunk (_stream_readable.js:273:9)\n' +
app_1       |       '    at Socket.Readable.push (_stream_readable.js:214:10)\n' +
app_1       |       '    at TCP.onStreamRead (internal/stream_base_commons.js:186:23)'
app_1       |   },
app_1       |   client: 'postgres',
app_1       |   table: 'users',
app_1       |   columns: [ 'email' ],
app_1       |   constraint: 'users_email_unique',
app_1       |   schema: undefined
app_1       | }

@robbyphillips
Copy link
Contributor

Is there a reason that the error handler doesn't look more like the objection recipe here: https://vincit.github.io/objection.js/recipes/error-handling.html#examples ?

@dekelev
Copy link
Member

dekelev commented Nov 19, 2020

You can always access the original error instance via a Symbol. check here for more info.

@GVanderLugt
Copy link
Author

@robbyphillips I'm wondering the same thing. It looks like the error handler is trying to handle Objection errors according to this comment. However, that block is never executing because error.statusCode is not defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants