fixed UniqueViolationError bug for mysql clients

[Tr3ffel]

Hi,

currently, I develop an app where users have an email which should be unique in the users table. So I added a unique index for the email column.

But when tried to save a user with an email which is already used, feathers-objection throws the following error: Cannot read property 'join' of undefined

After some research I found out that the feathers-objection error handler tries to generate an error message with the following code:

feathersError = new errors.Conflict(`${error.columns.join(', ')} must be unique`, {
  columns: error.columns,
  table: error.table,
  constraint: error.constraint
});

After some more research I found out that objection uses the db-error package. And as documented the UniqueViolationError does not support the columns field for mysql.

This probably causes error.columns to be undefined and error.columns.join(', ') will not work.

I fixed the bug by checking if the error.client is mysql and if so the native sqlMessage will be thrown. I also added a test for this.

I hope it helps.

[dekelev]

Thanks, I'll check it out

On Mon, Feb 22, 2021, 12:17 PM Marco Ahlers notifications@github.com wrote:

Hi,

currently, I develop an app where users have an email which should be unique in the users table. So I added a unique index for the email column.

But when tried to save a user with an email which is already used, feathers-objection throws the following error: Cannot read property 'join' of undefined

After some research I found out that the feathers-objection error handler tries to generate an error message with the following code:

feathersError = new errors.Conflict(${error.columns.join(', ')} must be unique, { columns: error.columns, table: error.table, constraint: error.constraint});

After some more research I found out that objection uses the db-error https://github.com/Vincit/db-errors package. And as documented https://github.com/Vincit/db-errors#uniqueviolationerror the UniqueViolationError does not support the columns field for mysql.

This probably causes error.columns to be undefined and error.columns.join(', ') will not work.

I fixed the bug by checking if the error.client is mysql and if so the native sqlMessage will be thrown. I also added a test for this.

I hope it helps.

You can view, comment on, or merge this pull request online at:

https://github.com/feathersjs-ecosystem/feathers-objection/pull/141 Commit Summary

• fixed UniqueViolationError bug when using mysql
• fixed a typo and added a test

File Changes

• M src/error-handler.js https://github.com/feathersjs-ecosystem/feathers-objection/pull/141/files#diff-0a502163240ffc42be6784b74dc879742fb2273986a469867b79889b64b36ffa (14)
• M test/index.test.js https://github.com/feathersjs-ecosystem/feathers-objection/pull/141/files#diff-af95cb6f5f65fe70216ddce70325e785438d5aaff956a0666ebf46ef6402f526 (11)

Patch Links:

- https://github.com/feathersjs-ecosystem/feathers-objection/pull/141.patch

https://github.com/feathersjs-ecosystem/feathers-objection/pull/141.diff

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/feathersjs-ecosystem/feathers-objection/pull/141, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB5E3NHQGJ2PJNFJHTQCZ3TAIVMDANCNFSM4YAHJNRA .

[dekelev]

@Tr3ffel Which version of objection NPM module do you use in your project?

According to Objection's docs, the current implementation should work regardless of the DB type. I haven't tested this myself on MySQL and the current tests definitely not suffice and do not protect from this type of errors (ObjectionJS throws unexpected error object). We must test this against real MySQL using the latest objection NPM module version before making any changes.

[Tr3ffel]

Hi,

currently, I use version 2.2.14.

I'm not sure want you mean by: ObjectionJS throws unexpected error object. I think this problem has nothing to do with Objection.js. Getting the table and columns field of an Unqiue Index Error just not seems to be possible for MySQL.

Also, I found a Pull-Request where this problem was already described.

[dekelev]

I see. So in ObjectionJS opinion, there's "no way to reliably get table and columns from mysql error". It makes sense, since MySQL doesn't return a unified error message with the table & columns like PostgreSQL & SQLite do and ObjectionJS seems to generally decided not to rely on the DB query itself for reference to the table & columns.

[dekelev]

Released with v7.1.2