Throw an error if a query parameter is a promise

[twavv]

I hope this is in scope.

It would be nice™ if node-postgres would throw an error if you try to use a promise as a parameter in a query. I just ran into an issue where I had a query like

// generateFoo returns a promise, but I forgot to await it here
const foo = generateFoo()
await db.query(`INSERT INTO mytable (foo) VALUE ($1)`, [foo])

and was surprised when I checked and the value of foo that was stored was, in fact, just the string "{}" - presumably from node-postgres trying to "cast" the promise object to send it to the database. It seems that JSONifying a Promise is not a thing that people would ever want to do, so it'd be nice if this pitfall could have been avoided.

Thanks for the project!

[vitaly-t]

The only way this could be done is by adding a configuration option to the library that would force the library to throw an error whenever a promise is to be returned from the query, for someone who wants to enforce the callback-only approach.

This would have made sense 3 years ago, but not today when await syntax has been supported for ages, and far a better approach to writing async code today.

In fact, since this library already requires NodeJS 8.0+, only the opposite would make sense - to retire the callback syntax completely, and thus get rid of that legacy ambiguity.

[sehrope]

@vitaly-t This isn't about the callback vs Promise return type of the query. This is in regards to the data type of query parameters passed in the params array. The driver has built-in handling for a bunch of types and defaults to stringifying anything else.

@travigd I've run into something similar to this when passing in Buffer-like objects that got (improperly) stringified rather than treating as a pre-serialized Buffer. I can see this being helpful for catching errors both for parameter args that are Promises and also functions:

const params = [
  'test',
  someAsyncFunc(), // <== Oops! I forgot to await the Promise
  someFuncThatWasNotInvoked, // <== Oops! I forgot to invoke the function
];
db.query(sql, params);

Not sure if this is even a breaking change as any existing code that does either of those things is broken anyway.

[twavv]

Yeah, @sehrope your interpretation of what I'm suggesting is correct. Sorry if my phrasing in the original issue was unclear! I'll edit it a little bit for clarity.