Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pool.query() does not throw error with full stack trace #1762

Closed
kevin-leptons opened this issue Nov 5, 2018 · 14 comments · Fixed by #2983
Closed

Pool.query() does not throw error with full stack trace #1762

kevin-leptons opened this issue Nov 5, 2018 · 14 comments · Fixed by #2983

Comments

@kevin-leptons
Copy link

Issue of Pool.query().

Errors which are throw form Pool.query() does not contains full error stack trace. Please throw standard error with full stack trace.

@joelmukuthu
Copy link

joelmukuthu commented Nov 5, 2018

Hi, could you post the incomplete stack trace and the code that generates the error? Without knowing more about your case however, I’m guessing that it might have something to do with this infamous bug in node/V8, which can’t be fixed in this library

@kevin-leptons
Copy link
Author

Hi Joe,

Yes, I got it, my case is similar like that. Now I know that we can't fix it in this library. What can we do in this case?, or just wait for node.js team?.

@joelmukuthu
Copy link

joelmukuthu commented Nov 5, 2018

You can try some of the workarounds suggested in that issue:

const bluebird = require('bluebird');
const { Client, Pool } = require('pg');

const pool = new Pool({
  Promise: bluebird
});

// for this one, only available in pg@7.6.0 and up:
const client = new Client({
  Promise: bluebird
});

@kevin-leptons
Copy link
Author

Thank Joel, I will try all of it. I also let this issue is open to show that it should be solve, some how.

@charmander
Copy link
Collaborator

@joelmukuthu’s covered it all; stack traces get lost by default in JavaScript when asynchronous tasks aren’t direct calls.

@phiresky
Copy link
Contributor

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 node-pg, maybe because node-pg doesn't use native promises or native async functions?

@brianc
Copy link
Owner

brianc commented Mar 30, 2020

I'm not sure what the reason is that this is still broken in node-pg

the reason is this project has been maintained almost entirely by myself and 1 other person for the past decade and we're buried under bug reports, feature requests, internal priorities. Also, the world has fallen into a global pandemic causing me significant anxiety attacks. Also, we still support versions of node back to 6.0.

@brianc
Copy link
Owner

brianc commented Mar 30, 2020

maybe because node-pg doesn't use native promises or native async functions?

with the 9.0 internals re-write i'll be starting as soon as I can catch my emotional breath I'm going to investigate switching fully async/await (as much as possible) internally. We'll see how the perf of that is, but if that works you might just get this (and every other feature ever since it's open source) for free!

@phiresky
Copy link
Contributor

No problem, sorry that I appeared to be demanding stuff for free. Mainly I think this issue could be reopened, since the original reason where this was not possible at all due to node restrictions is gone.

@brianc
Copy link
Owner

brianc commented Mar 30, 2020 via email

@tolmekian1453
Copy link

tolmekian1453 commented May 3, 2020

Not sure if I'm doing something wrong, but nothing in the above conversation or similar ones elsewhere worked.

This is what I did, works and doesn't require more dependencies. Dunno if it'll work on older Node versions.

const executeSql = async function(conn, q, args) {
  // gets the stack before the error, otherwise it's the emitResult stuff
  const stack = new Error().stack 
  try {
    return await conn.query(q, args)
  } catch(err) {
    console.error(stack)
    throw err
  }
}

@mcapodici
Copy link

mcapodici commented Jul 1, 2020

I'm not sure what the reason is that this is still broken in node-pg

the reason is this project has been maintained almost entirely by myself and 1 other person for the past decade and we're buried under bug reports, feature requests, internal priorities. Also, the world has fallen into a global pandemic causing me significant anxiety attacks. Also, we still support versions of node back to 6.0.

A big thank you. I don't know how popular FOSS maintainers do it all 👍 .

I have the same problem. I am scared to use different types of promises and watnot. I am just going to try console logging before any DB operation, then I can see roughly when the EX happened. Obviously my use case is low volume!

@pauldraper
Copy link

pauldraper commented May 6, 2021

Also, we still support versions of node back to 6.0.

Props, though given that Node 6 was EOL'd April 2019, that's just crazy.

At the time of this writing (May 2021), the oldest maintained LTS version is Node.js 12.

@phiresky
Copy link
Contributor

I've made a PR to fix this: #2983

brianc pushed a commit that referenced this issue May 31, 2023
* fix stack traces of query() to include the async context (#1762)

* rename tests so they are actually run

* conditionally only run async stack trace tests on node 16+

* add stack trace to pg-native

---------

Co-authored-by: Charmander <~@charmander.me>
thijs pushed a commit to thijs/node-postgres that referenced this issue Aug 1, 2023
brianc#2983)

* fix stack traces of query() to include the async context (brianc#1762)

* rename tests so they are actually run

* conditionally only run async stack trace tests on node 16+

* add stack trace to pg-native

---------

Co-authored-by: Charmander <~@charmander.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants