brianc / node-postgres-docs

Documentation for node-postgres
https://node-postgres.com
MIT License
98 stars 94 forks source link

Need to close the cursor in case of an error? #172

Open lukaromih opened 2 years ago

lukaromih commented 2 years ago

The pg.Cursor docs aren't clear enough when we (the users of the pg and pg-cursor packages) should explicitly call the cursor.close method to free the resources.

As somebody who's not too experienced with Postgres or SQL cursors in general, I had to read through Postgres docs, to now imagine that cursors are automatically closed and their resources cleared, when they reach the end, though I'm in no way certain of it, that's just my current interpretation of what I've read.

The pg.Cursor API docs show an example of reading the first 100 rows from the cursor, then closing it and releasing the client after. And the .close method is described as "Used to close the cursor early. If you want to stop reading from the cursor before you get all of the rows returned, call this."

The error handling with cursors is not documented well enough. There are 2 examples, both callback based. The first one doesn't account for and handle errors at all (for brevity sake, I imagine). The second one just throws in case of an error, which, I imagine, would stop the entire process and thus release the client anyway.

I'm using the cursor as part of a bigger solution where I don't want to kill the whole process and I want to handle the error gracefully. Also I'm using the async/await syntax. This involves acquiring the client then wrapping the whole logic into try/finally, where I release the client in the finally block. Should I call cursor.close() inside a catch block in case of error, or do errors close the cursor internally? Should I call it in finally? Is it unnecessary since I want to read the whole cursor anyway?

I did some inspecting of the pg-cursor package source code, but I can't really be 100%. Also, whenever I find myself having to inspect source code of some dependency, I imagine that's an indicator of sub-optimal documentation.