This was brought up to a degree in #166, but I don't think was followed on because the original reporter solved his _specific_ problem, but the core problem with the stack traces remains. Here's a really simple test case:
let db = require('pg-promise')()(require('config').rds);
db.query('select error').catch(err => console.error(err));
I would expect that the stack trace would tell me what part of the query fails (it does), as well as from where the query was invoked (it doesn't). Not having the latter makes it REALLY hard to troubleshoot in anything but a simple app or test case. Here's the output of the above test case (directories sanitized):
โฏ node ./pg-promise-test.js
{ error: column "error" does not exist
at Connection.parseE (.../node_modules/pg-promise/node_modules/pg/lib/connection.js:539:11)
at Connection.parseMessage (.../node_modules/pg-promise/node_modules/pg/lib/connection.js:366:17)
at Socket.<anonymous> (.../node_modules/pg-promise/node_modules/pg/lib/connection.js:105:22)
at emitOne (events.js:96:13)
at Socket.emit (events.js:191:7)
at readableAddChunk (_stream_readable.js:178:18)
at Socket.Readable.push (_stream_readable.js:136:10)
at TCP.onread (net.js:561:20)
name: 'error',
length: 97,
severity: 'ERROR',
code: '42703',
detail: undefined,
hint: undefined,
position: '8',
internalPosition: undefined,
internalQuery: undefined,
where: undefined,
schema: undefined,
table: undefined,
column: undefined,
dataType: undefined,
constraint: undefined,
file: 'parse_relation.c',
line: '2892',
routine: 'errorMissingColumn' }
You'll notice that nowhere in the stack trace does it actually reference the line in the test case that actually caused the error, making the source of the problem damn near impossible to find in non-trivial cases. This is not an issue when using the callback-based approach in node-postgres.
Are there plans to actually handle this with native Promises so I don't have to pull in another Promise lib just to get tack traces out of pg-promise?
Are there plans to actually handle this with native Promises
Native promises, aka ES6 Promises are part of Node.js since v0.11, not this library.
so I don't have to pull in another Promise lib just to get tack traces out of pg-promise
Bluebird isn't just another promise library. It is The promise library that should be used today, also the only one that does stack tracing so well.
The standard ES6 Promise is too basic anyway, don't expect much from it.
So the answer then is "no" you won't be supporting ES6 Promises with useful stack traces ever and the only way to get them is by pulling in bluebird as pg-promise's Promise lib? Just making sure I'm understanding you here. If this is the case you might want to note it in your docs for promiseLib since this was a major problem for me and I assume many others.
You understand this almost correctly, except the part where you're seemingly blaming this library for not supporting or documenting it.
Stack tracing for promises is an area that's supposed to be addressed by the promise library, that's the way it's always been. I guess it takes a little practice to figure out those kind of basics. I can't really cover all of them in my documentation.
As far as this library is concerned, it supports all existing promise libraries, which is well-documented. And that's the main thing.
Dude, relax, you seem to be just oozing with condescension, insinuating that I have no idea how Promises work. I'm just trying to highlight a pain point that you yourself are aware. I'm suggesting that if you KNOW bluebird with long stack traces is the only way to get useful stack traces out of pg-promise, it MIGHT be worth noting in your section of documentation dedicated to Promise lib support. You don't have to take this suggestion as a persona affront, it's just feedback from a user, not an attack on your project or prowess as a developer. Do with my suggestion what you will.
There may be other promise libraries beside Bluebird today that support long stack traces, I don't know, I haven't looked into it in a while. But I wouldn't suggest any alternatives anyway.
And I do accept PR-s that improve documentation :wink:
Not so much noting that bluebird supports long stack traces, but that ES6 stack traces (default) WON'T be helpful and that you should use an alternative lib to get them. You're right, stack traces are the realm of the Promise implementation, but not all libs are affected as negatively as pg-promise appears to be WRT stack traces, hence my initial concern.
Happy to PR, so long as you can drop the condescending comments. I'd much rather be a contributor than some unfairly perceived insurgent on your repo.
Happy to PR, so long as you can drop the condescending comments
I'll do my best ๐
I'm going to have to give round one to @tonylukasavage I am looking forward to round 2 "The Pull Request" ๐๐
@euforic you are 7 hours too late, this was the PR.
Some of us had to work haha. Glad to see it all worked out. ๐ฌ
@tonylukasavage to make you feel better, following this issue I brought this up within the Massive.js that just migrated to pg-promise: New driver considerations.
So the word is spreading, even if it costs you some hair :smile:
๐
On Jun 3, 2017 3:35 PM, "Vitaly Tomilov" notifications@github.com wrote:
@tonylukasavage https://github.com/tonylukasavage to make you feel
better, following this issue I brought this up within the Massive.js that
just migrated to pg-promise: New driver considerations
https://github.com/dmfay/massive-js/issues/381.โ
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/vitaly-t/pg-promise/issues/342#issuecomment-305996697,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAkQwNn9ybWPoVcym6M4PErvBBROqXEyks5sAbWIgaJpZM4NsK9-
.
My grain of salt in case someone has the same problem.
I didn't liked just to extend the stack using an external lib so I just did this in my queries.js files
so i just replaced the stack, I don't think i need the stack of the library
async function getAll() {
try {
const users = await dbClient.db.any(`
SELECT ${memberFields} FROM member
`)
return users
} catch (err) {
throw new Error(err.message)
}
}
I guess you can also do something to call a method and wrap it in a try catch that you end up using something like
function getAll() {
return patchedClient.any(`
SELECT ${memberFields} FROM member
`)
}
and this patched client "any" function has the try and catch logic inside it, and returns a promise that you can await
@vitaly-t do you see any potencial problem in doing this?
@psypersky
I didn't liked just to extend the stack using an external lib
That is the only practical way today. And what is there not to like? It works perfectly.
do you see any potencial problem in doing this?
A few:
Beside this, you also should use the library's query formatting, and not ES6 template strings, i.e. SELECT ${memberFields} should rely on use of filter :name instead.
@vitaly-t I wonder is it possible to improve the current situation with async stack traces that are part of node 12. After upgrading stack traces are still not readable and I cant understand why is that ๐ค
@krzkaczor
As per the older posts, just use the Bluebird, via option promiseLib, and enable Long Stack Traces.
@vitaly-t Thanks, I get it, that's always a possibility, it's just node's async stack traces have 0
performance impact and are enabled by default. So it's much more accessible for regular users.
Now that zero-cost async stack traces are part of Node (v12) by default, it would be great if this could be reconsidered. I'm not sure what the reason is that this is still broken in pg-promise, maybe pg-promise doesn't use native promises or native async?
@phiresky What you referenced here is not even relevant. As of today, NodeJS still does not properly support long-stack tracing.
this is still broken in pg-promise
Still broken in pg-promise? This was never broken to begin with. You set which promise library you want to use, and from there how to make it do long stack tracing.
I am locking this conversation, as there is nothing of value to be added here.
Most helpful comment
@vitaly-t Thanks, I get it, that's always a possibility, it's just node's async stack traces have 0
performance impact and are enabled by default. So it's much more accessible for regular users.