Duplicate validation is failing and returning a 500 error instead of 400

[kieron]

Bug report

Duplicate validation is failing and returning a 500 error instead of 400

Steps to reproduce the behaviour

1. Submit duplicate data

Expected behaviour

Should return 400 error

System

• Node.js version: v12.17.0
• NPM version: 6.14.4
• Strapi version: v3.1.7
• Database: MySQL + bookshelf
• Operating system: Centos

[derrickmehaffy]

I was able to reproduce this on v3.3.3.
Test project:
test8664.zip

To reproduce

• Download the sample project
• Install/build/run
• Default admin login is test@test.com and Test1234!
• See existing entry in test model with a value of https://gleam.io/sometesturl
• Try to send a POST request with:

{
    "gleam_url": "https://gleam.io/sometesturl"
}

• Notice 500 error instead of 400, the duplicate validation is failing and returning a 500 error that the user can't see why. Tested on both SQLite (test project) and MariaDB 10.5

SQLite error:

[2020-11-16T21:35:20.748Z] error Error: SQLITE_CONSTRAINT: UNIQUE constraint failed: tests.gleam_url
    at Client_SQLite3._query (/home/dmehaffy/Development/testDup/node_modules/knex/lib/dialects/sqlite3/index.js:99:12)
    at Client_SQLite3.query (/home/dmehaffy/Development/testDup/node_modules/knex/lib/client.js:158:17)
    at /home/dmehaffy/Development/testDup/node_modules/knex/lib/transaction.js:331:24
From previous event:
    at Client_SQLite3.trxClient.query (/home/dmehaffy/Development/testDup/node_modules/knex/lib/transaction.js:326:12)
    at Runner.query (/home/dmehaffy/Development/testDup/node_modules/knex/lib/runner.js:135:36)
    at /home/dmehaffy/Development/testDup/node_modules/knex/lib/runner.js:39:23
From previous event:
    at Runner.run (/home/dmehaffy/Development/testDup/node_modules/knex/lib/runner.js:25:16)
    at Builder.Target.then (/home/dmehaffy/Development/testDup/node_modules/knex/lib/interface.js:14:43)
    at Child.<anonymous> (/home/dmehaffy/Development/testDup/node_modules/bookshelf/lib/model.js:1160:42)
From previous event:
    at Child.<anonymous> (/home/dmehaffy/Development/testDup/node_modules/bookshelf/lib/model.js:1159:14)
From previous event:
    at Child.<anonymous> (/home/dmehaffy/Development/testDup/node_modules/bookshelf/lib/model.js:1060:10)
From previous event:
    at runCreate (/home/dmehaffy/Development/testDup/node_modules/strapi-connector-bookshelf/lib/queries.js:98:45)
    at /home/dmehaffy/Development/testDup/node_modules/strapi-connector-bookshelf/lib/queries.js:46:34
    at /home/dmehaffy/Development/testDup/node_modules/knex/lib/transaction.js:91:24
From previous event:
    at /home/dmehaffy/Development/testDup/node_modules/knex/lib/transaction.js:77:12
    at processImmediate (internal/timers.js:461:21)
From previous event:
    at new Transaction (/home/dmehaffy/Development/testDup/node_modules/knex/lib/transaction.js:57:30)
    at Client_SQLite3.transaction (/home/dmehaffy/Development/testDup/node_modules/knex/lib/client.js:114:12)
    at Function.transaction (/home/dmehaffy/Development/testDup/node_modules/knex/lib/util/make-knex.js:40:31)
    at wrapTransaction (/home/dmehaffy/Development/testDup/node_modules/strapi-connector-bookshelf/lib/queries.js:46:15)
    at Object.create (/home/dmehaffy/Development/testDup/node_modules/strapi-connector-bookshelf/lib/queries.js:105:12)
    at fn (/home/dmehaffy/Development/testDup/node_modules/strapi-database/lib/queries/helpers.js:31:54)
    at Object.create (/home/dmehaffy/Development/testDup/node_modules/strapi-database/lib/queries/helpers.js:15:24)
    at async Object.create (/home/dmehaffy/Development/testDup/node_modules/strapi/lib/services/entity-service.js:75:17)
    at async Object.create (/home/dmehaffy/Development/testDup/node_modules/strapi/lib/core-api/controller.js:123:18)
    at async /home/dmehaffy/Development/testDup/node_modules/strapi/lib/middlewares/router/utils/routerChecker.js:79:22
    at async module.exports (/home/dmehaffy/Development/testDup/node_modules/strapi-plugin-users-permissions/config/policies/permissions.js:86:3)
    at async /home/dmehaffy/Development/testDup/node_modules/strapi-utils/lib/policy.js:68:5
    at async /home/dmehaffy/Development/testDup/node_modules/strapi/lib/middlewares/parser/index.js:48:23
    at async /home/dmehaffy/Development/testDup/node_modules/strapi/lib/middlewares/xss/index.js:26:9
[2020-11-16T21:35:20.749Z] debug POST /tests (31 ms) 500

[derrickmehaffy]

This issue has been mentioned on Strapi Community. There might be relevant details there:

https://forum.strapi.io/t/could-the-duplicate-entry-error-be-sent-back-in-the-web-request/1069/3

[derrickmehaffy]

@kieronbos do you want to try and apply this fix manually? (I plan to do some testing also)

#8367

[AlphaMoonbaseBerlin]

It is a simple hack for now, but putting the service call in a try/catch block and returning the error as the message gives a more friendly warning (only works when you setup your own endpoints

catch (e) {
            entity = {  statusCode: 400,
                        error :   "Bad Request",
                        message :   e};
            ctx.response.status = 400;
        }

[derrickmehaffy]

Additional information since this is throwing 500 errors to REST/GraphQL clients:

Original PR that fixed it in the Admin: #8367

Related code that is throwing the errors:

MongoDB (Mongoose)

strapi/packages/strapi-connector-mongoose/lib/utils/errors.js

Lines 7 to 13 in 86e0cf0

	const handleDatabaseError = error => {
	if (isDuplicateEntryError(error)) {
	strapi.log.warn('Duplicate entry', error.toString());
	throw new Error('Duplicate entry');
	}
	throw error;
	};

strapi/packages/strapi-connector-mongoose/lib/queries.js

Lines 62 to 68 in 86e0cf0

	const wrapErrors = fn => async (...args) => {
	try {
	return await fn(...args);
	} catch (error) {
	return handleDatabaseError(error);
	}
	};

SQL (Bookshelf)

strapi/packages/strapi-connector-bookshelf/lib/utils/errors.js

Lines 22 to 28 in 7271c1a

	const handleDatabaseError = error => {
	if (isDuplicateEntryError(error)) {
	strapi.log.warn('Duplicate entry', error.toString());
	throw new Error('Duplicate entry');
	}
	throw error;
	};

strapi/packages/strapi-connector-bookshelf/lib/queries.js

Lines 54 to 60 in 86e0cf0

	const wrapErrors = fn => async (...args) => {
	try {
	return await fn(...args);
	} catch (error) {
	return handleDatabaseError(error);
	}
	};

[derrickmehaffy]

The above code doesn't return a proper error to the user (400 status code + proper error)

[christher-lenander]

Same problem with the MySQL. And still not fixed.