feat: make connect() require that connection is contained within a pr…

…omise routine

BREAKING CHANGE:
* `pool#connect()` no longer returns a connection. Refer to “Checking out a client from the connection pool” documentation for new API and reasoning behind the change.

.README/README.md

@@ -10,17 +10,19 @@ A PostgreSQL client with strict types, detail logging and assertions.
 
 ## Features
 
-* [Convenience methods](#slonik-query-methods) with built-in assertions
-* Anonymous, named and tagged template literal [value placeholders](#slonik-value-placeholders)
-* [Middleware](#slonik-interceptors) support
-* [Syntax highlighting](#slonik-syntax-highlighting) (Atom plugin compatible with Slonik)
-* [SQL injection guarding](https://github.com/gajus/eslint-plugin-sql) (ESLint plugin compatible with Slonik)
-* Detail [logging](#slonik-debugging)
-* [Parsing and logging of the auto_explain logs.](#logging-auto_explain)
-* Built-in [asynchronous stack trace resolution](#log-stack-trace)
-* [Flow types](#types)
-* [Mapped errors](#error-handling)
-* [Transactions](#transactions)
+* Predominantly compatible with [node-postgres](https://github.com/brianc/node-postgres) (see [Incompatibilities with `node-postgres`](#incompatibilities-with-node-postgres)).
+* [Convenience methods](#slonik-query-methods) with built-in assertions.
+* Anonymous, named and tagged template literal [value placeholders](#slonik-value-placeholders).
+* [Middleware](#slonik-interceptors) support.
+* [Syntax highlighting](#slonik-syntax-highlighting) (Atom plugin compatible with Slonik).
+* [SQL injection guarding](https://github.com/gajus/eslint-plugin-sql) (ESLint plugin compatible with Slonik).
+* Detail [logging](#slonik-debugging).
+* [Parsing and logging of the auto_explain logs.](#logging-auto_explain).
+* Built-in [asynchronous stack trace resolution](#log-stack-trace).
+* [Safe connection pooling](#checking-out-a-client-from-the-connection-pool).
+* [Flow types](#types).
+* [Mapped errors](#error-handling).
+* [Transactions](#transactions).
 
 ---
 
@@ -34,9 +36,10 @@ A PostgreSQL client with strict types, detail logging and assertions.
 
 {"gitdown": "include", "file": "./RECIPES.md"}
 
-## Non-standard behaviour
+## Incompatibilities with `node-postgres`
 
 * `timestamp` and `timestamp with time zone` returns UNIX timestamp in milliseconds.
+* Connection pool `connect()` method requires that connection is restricted to a single promise routine (see [Checking out a client from the connection pool](#checking-out-a-client-from-the-connection-pool)).
 
 ## Conventions
 

.README/USAGE.md

@@ -49,8 +49,75 @@ import {
   createPool
 } from 'slonik';
 
-const connection = createPool('postgres://localhost');
+const pool = createPool('postgres://localhost');
 
-await connection.query(sql`SELECT 1`);
+await pool.query(sql`SELECT 1`);
 
 ```
+
+### Checking out a client from the connection pool
+
+Slonik only allows to check out a connection for a duration of promise routine supplied to the `connect()` method.
+
+```js
+import {
+  createPool
+} from 'slonik';
+
+const pool = createPool('postgres://localhost');
+
+const result = await pool.connect(async (connection) => {
+  await connection.query(sql`SELECT 1`);
+  await connection.query(sql`SELECT 2`);
+
+  return 'foo';
+});
+
+result;
+// 'foo'
+
+```
+
+Connection is released back to the pool after the promise produced by the function supplied to `connect()` method is either resolved or rejected.
+
+The primary reason for implementing _only_ this connection pooling method is because the alternative is inherently unsafe, e.g.
+
+```js
+// Note: This example is using unsupported API.
+
+const main = async () => {
+  const connection = await pool.connect();
+
+  await connection.query(sql`SELECT produce_error()`);
+
+  await connection.release();
+};
+
+```
+
+In this example, the error causes early rejection of the promise and a hanging connection. A fix to the above is to ensure that `connection#release()` is always called, i.e.
+
+```js
+// Note: This example is using unsupported API.
+
+const main = async () => {
+  const connection = await pool.connect();
+
+  let lastExecutionResult;
+
+  try {
+    lastExecutionResult = await connection.query(sql`SELECT produce_error()`);
+  } catch (error) {
+    await connection.release();
+
+    throw error;
+  }
+
+  await connection.release();
+
+  return lastExecutionResult;
+};
+
+```
+
+Slonik abstracts the latter pattern into `pool#connect()` method.

src/binders/bindPool.js

@@ -32,7 +32,7 @@ export default (
   return {
     any: mapTaggedTemplateLiteralInvocation(any.bind(null, parentLog, pool, clientConfiguration)),
     anyFirst: mapTaggedTemplateLiteralInvocation(anyFirst.bind(null, parentLog, pool, clientConfiguration)),
-    connect: async () => {
+    connect: async (connectionRoutine) => {
       const connection: InternalDatabaseConnectionType = await pool.connect();
 
       const bindedConnection = bindPoolConnection(parentLog, pool, connection, clientConfiguration);
@@ -41,7 +41,17 @@ export default (
         await clientConfiguration.onConnect(bindedConnection);
       }
 
-      return bindedConnection;
+      let result;
+
+      try {
+        result = await connectionRoutine(bindedConnection);
+      } catch (error) {
+        await connection.release();
+
+        throw error;
+      }
+
+      return result;
     },
     many: mapTaggedTemplateLiteralInvocation(many.bind(null, parentLog, pool, clientConfiguration)),
     manyFirst: mapTaggedTemplateLiteralInvocation(manyFirst.bind(null, parentLog, pool, clientConfiguration)),

src/binders/bindPoolConnection.js

@@ -40,9 +40,6 @@ export default (
     one: mapTaggedTemplateLiteralInvocation(one.bind(null, parentLog, pool, clientConfiguration)),
     oneFirst: mapTaggedTemplateLiteralInvocation(oneFirst.bind(null, parentLog, pool, clientConfiguration)),
     query: mapTaggedTemplateLiteralInvocation(query.bind(null, parentLog, pool, clientConfiguration)),
-    release: async () => {
-      connection.release();
-    },
     transaction: async (handler: TransactionFunctionType) => {
       return createPoolTransaction(parentLog, pool, clientConfiguration, handler);
     }

src/types.js

@@ -88,13 +88,14 @@ export type DatabaseSingleConnectionType = {|
 
 export type DatabasePoolConnectionType = {|
   ...CommonQueryMethodsType,
-  +release: () => Promise<void>,
   +transaction: (handler: TransactionFunctionType) => Promise<*>
 |};
 
+export type ConnectionRoutineType = (connection: DatabasePoolConnectionType) => Promise<*>;
+
 export type DatabasePoolType = {|
   ...CommonQueryMethodsType,
-  +connect: () => Promise<DatabasePoolConnectionType>,
+  +connect: (connectionRoutine: ConnectionRoutineType) => Promise<*>,
   +transaction: (handler: TransactionFunctionType) => Promise<*>
 |};