Bad request to blueprint's action causes 500 error

Sails version: 0.11.4
Node version: v5.4.1
NPM version: 3.5.2
Operating system: Windows 10 64-bit

---

When I try to request blueprint action route with invalid data it responses with Internal Server Error.
Let's say we have an Account model and want to fetch data through the API. We use Blueprint's route GET /account/:id, but we accidentaly sent a username string instead of id. Sails reponse is:

{
    "error": "E_UNKNOWN",
    "status": 500,
    "summary": "Encountered an unexpected error",
    "raw": {
        "code": "ER_BAD_FIELD_ERROR",
        "errno": 1054,
        "sqlState": "42S22",
        "index": 0
    }
}

Which I believe should result in bad request. You know, I hate getting server errors in production.

EDIT:
I'm using sails-mysql adapter, here's the call stack

ER_BAD_FIELD_ERROR: Unknown column 'NaN' in 'where clause'
    at Query.Sequence._packetToError (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\sequences\Sequence.js:48:14)
    at Query.ErrorPacket (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\sequences\Query.js:83:18)
    at Protocol._parsePacket (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Protocol.js:274:23)
    at Parser.write (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Parser.js:77:12)
    at Protocol.write (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Protocol.js:39:16)
    at Socket.<anonymous> (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\Connection.js:96:28)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at readableAddChunk (_stream_readable.js:146:16)
    at Socket.Readable.push (_stream_readable.js:110:10)
    at TCP.onread (net.js:523:20)
    --------------------
    at Protocol._enqueue (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Protocol.js:135:48)
    at PoolConnection.query (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\Connection.js:201:25)
    at __FIND__ (D:\_htdocs\jargames2\node_modules\sails-mysql\lib\adapter.js:834:20)
    at afterwards (D:\_htdocs\jargames2\node_modules\sails-mysql\lib\connections\spawn.js:84:5)
    at D:\_htdocs\jargames2\node_modules\sails-mysql\lib\connections\spawn.js:40:7
    at Ping.onOperationComplete [as _callback] (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\Pool.js:99:5)
    at Ping.Sequence.end (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\sequences\Sequence.js:96:24)
    at Ping.Sequence.OkPacket (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\sequences\Sequence.js:105:8)
    at Protocol._parsePacket (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Protocol.js:274:23)
    at Parser.write (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Parser.js:77:12)
    at Protocol.write (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\protocol\Protocol.js:39:16)
    at Socket.<anonymous> (D:\_htdocs\jargames2\node_modules\sails-mysql\node_modules\mysql\lib\Connection.js:96:28)
    at emitOne (events.js:77:13)
    at Socket.emit (events.js:169:7)
    at readableAddChunk (_stream_readable.js:146:16)
    at Socket.Readable.push (_stream_readable.js:110:10)
    at TCP.onread (net.js:523:20)

[sgress454]

Thanks @selenir. I think this problem has been brought up in the past under different guises, and never quite addressed.

#3258
#2976

I'll see if I can shake some trees and see if any progress has been made.

[BrownBear2]

This is an annoying bug. I think, the coercion function coercePK in waterline/utils/normalize.js should catch NaN. If it coerced to 0 if NaN or +/-Infinity, the problem would be fixed.

Any progress on this one?

[rmzr7]

Any updates on this? @mikermcneil

[mikermcneil]

@BrownBear2 Thanks for the PR, I followed up @particlebanana's review with one of my own and left a note balderdashy/waterline#1351 (review). If you wouldn't mind making an identical PR to the 0.11 branch of Waterline as well, I'd appreciate it (then we can do a patch release of Waterline 0.11 as soon as that's in place, which will solve this for Sails v0.11 and v0.12 apps immediately.)

@remzr7 @selenir As an update: We've been working on type safety in queries extensively for Waterline 0.13/Sails v1.0. There'll be a breaking change to the way you negotiate errors like this, but it'll also come with much more thorough verification of all incoming Waterline queries. Here are some of the notes I've put together on the subject: https://github.com/balderdashy/waterline/blob/bd92bd3f62cd242e3aed29d605b829d3d2027eef/ARCHITECTURE.md#querying-implementation

[mikermcneil]

@BrownBear2 @selenir @remzr7 Spent a lot of time on this in Waterline recently (see #3459 (comment) for a quick overview or https://github.com/balderdashy/waterline/tree/045891939c24ab21fa425ba5f251c99c86012532/lib/waterline/utils/query for the nitty gritty)

For posterity, here's an example of one of the places it's handled in blueprints:

sails/lib/hooks/blueprints/actions/update.js

Lines 76 to 89 in e544cfb

	// Differentiate between waterline-originated validation errors
	// and serious underlying issues. Respond with badRequest if a
	// validation error is encountered, w/ validation info.
	if (err) {
	switch (err.name) {
	case 'AdapterError':
	switch (err.code) {
	case 'E_UNIQUE': return res.badRequest(err);
	default: return res.serverError(err);
	} return;
	case 'UsageError': return res.badRequest(err);
	default: return res.serverError(err);
	}
	}//-•