Improve bindings debug information when a binding in the where clause is missing.

[vincentheet]

I just upgraded our project from knex 0.10.0 to 0.11.7 and noticed the new bindings check on the where clause, great work but I think it can be improved.

[2016-07-06T12:03:13.983Z] ERROR: u-rest-js/13896 on Macintosh-128k.local: Undefined binding(s) detected when compiling SELECT query: select "Student".* from "Student" where "Student"."id" = ? and Student.removed = ? limit ?
Error: Undefined binding(s) detected when compiling SELECT query: select "Student".* from "Student" where "Student"."id" = ? and Student.removed = ? limit ?
at QueryCompiler_SQLite3.toSQL (/Users/user/GIT/u-base/node_modules/knex/lib/query/compiler.js:78:13)

Since i'm running in debug mode it would be useful to have a bindings array to debug which binding is missing in the query. Maybe the standard debug object can be added to the error? This was the debug object I got using knex 0.10.0 which makes it clear for me that "Student"."id" is undefined and that is what I should fix.

{ method: 'select',
options: {},
bindings: [ undefined, 0, 1 ],
sql: 'select "Student".* from "Student" where "Student"."id" = ? and Student.removed = ? limit ?' }

[wubzz]

This needs some extra thought before implementing something. I intentionally only included the actual query string due to cases where you may have a massive query with tons of bindings. I agree bindings would be great to have in there tho. Preferably the .toString() of the querybuilder would be the best, but that too will throw this error. 😏

Another idea I had in mind, which im not sure is really optimal, is to create an "initiation stack" when a query builder is initiated. That way we could technically even point directly to the part of the users code that is sending undefined values to knex. Maybe this is overkill tho.

Would work by appending this to the Raw's constructor (and any other builder)

Error.captureStackTrace(this, this.constructor)

[zacronos]

This would be great to have, and the initiation stack would be AMAZING -- it can be really frustrating to have knex tell me something is wrong, but not give me either the data I passed in or a stack trace showing where in my code to look. I have a very large webapp with lots of moving parts and backend supporting tasks, so I wind up having to look at the unbound query string output and think backwards: "where in the code do we do something like this?", and sometimes have to check a few different places since I don't have the data bindings to help.

I would caution about calling Error.captureStackTrace every time a query builder is initiated, however -- I believe there is significant overhead to that call. If it could be toggled on as an option for debugging though, it would be great.

[zacronos]

I've thought a bit more about this (due to use cases for it occurring during my work), and if there is enough concern about keeping large numbers of bindings around that the default needs to be the current behavior of NOT keeping the bindings, then maybe it should be an initialization setting. Unless there's some overhead I'm not thinking about, I know in my code we never have so many bindings that their usefulness for troubleshooting would be trumped by the performance hit.

Really there are 2 separate feature requests here, but both serve the goal of making knex errors easier to troubleshoot when you don't know what query/code actually caused the error: data bindings and initiation stack. I think they would both be VERY useful enhancements -- the initiation stack would be useful in situations where the data bindings aren't the issue, though the added overhead is higher. My suggestion is to make each of them a separate initialization setting. This is because honestly both of those features would, I think, be useful far more often than simply turning on full debug: true and seeing all queries, or turning on a knex:<something> logger; these new options would just be used to enhance the info on error objects. Something like dataBindingsOnError: true and reportInitiationStack: true.

[jeffreybrowning]

I am also having issues debugging raw queries without being given the parameters. It's a real headache.

[wubzz]

The bindings are since 2016 logged whenDEBUG=knex:bindings / DEBUG=knex:*. I know this doesn't solve much for production use.

I don't think adding the bindings in plaintext to the error message is such a good idea since generally error messages will be logged to some error log file and the bindings can contain very sensitive information in some cases. Showing the SQL without bindings at least does not expose any sensitive information.

Stacktraces is an issue in and of itself and is an area where we can maybe improve a bit. Currently being discussed in terms of async/await in #2500 .