Make error handling more explicit, and warn user if they haven't (#510)

Author: benjie

RELEASE_NOTES.md

@@ -31,6 +31,8 @@ Read more:
   clearer that it's for use in one-off locations (some felt the "quick" referred
   to the speed it executed, rather than the amount of effort required from the
   programmer)
+- We'll now warn you if you haven't installed error handlers on the pool, and
+  will only install them ourself if needed
 - Fixes bug where CLI defaults override `graphile.config.js` settings (by
   removing CLI defaults)
 - Fix bug where executable tasks had their stdout/stderr ignored; this is now

src/lib.ts

@@ -1,7 +1,7 @@
 import * as assert from "assert";
 import { EventEmitter } from "events";
 import { applyHooks, AsyncHooks, resolvePresets } from "graphile-config";
-import { Client, Pool, PoolClient } from "pg";
+import { Client, Pool, PoolClient, PoolConfig } from "pg";
 
 import { makeWorkerPresetWorkerOptions } from "./config";
 import { migrations } from "./generated/sql";
@@ -284,7 +284,6 @@ export async function assertPool(
   releasers: Releasers,
 ): Promise<Pool> {
   const {
-    logger,
     resolvedPreset: {
       worker: { maxPoolSize, connectionString },
     },
@@ -299,28 +298,50 @@ export async function assertPool(
   let pgPool: Pool;
   if (_rawOptions.pgPool) {
     pgPool = _rawOptions.pgPool;
+    if (pgPool.listeners("error").length === 0) {
+      console.warn(
+        `Your pool doesn't have error handlers! See: https://err.red/wpeh`,
+      );
+      installErrorHandlers(compiledSharedOptions, releasers, pgPool);
+    }
   } else if (connectionString) {
-    pgPool = new Pool({
+    pgPool = makeNewPool(compiledSharedOptions, releasers, {
       connectionString,
       max: maxPoolSize,
     });
-    releasers.push(() => {
-      pgPool.end();
-    });
   } else if (process.env.PGDATABASE) {
-    pgPool = new Pool({
+    pgPool = makeNewPool(compiledSharedOptions, releasers, {
       /* Pool automatically pulls settings from envvars */
       max: maxPoolSize,
     });
-    releasers.push(() => {
-      pgPool.end();
-    });
   } else {
     throw new Error(
       "You must either specify `pgPool` or `connectionString`, or you must make the `DATABASE_URL` or `PG*` environmental variables available.",
     );
   }
 
+  return pgPool;
+}
+
+function makeNewPool(
+  compiledSharedOptions: CompiledSharedOptions,
+  releasers: Releasers,
+  poolOptions: PoolConfig,
+) {
+  const pgPool = new Pool(poolOptions);
+  releasers.push(() => {
+    pgPool.end();
+  });
+  installErrorHandlers(compiledSharedOptions, releasers, pgPool);
+  return pgPool;
+}
+
+function installErrorHandlers(
+  compiledSharedOptions: CompiledSharedOptions,
+  releasers: Releasers,
+  pgPool: Pool,
+) {
+  const { logger } = compiledSharedOptions;
   const handlePoolError = (err: Error) => {
     /*
      * This handler is required so that client connection errors on clients
@@ -358,7 +379,6 @@ export async function assertPool(
     pgPool.removeListener("error", handlePoolError);
     pgPool.removeListener("connect", handlePoolConnect);
   });
-  return pgPool;
 }
 
 export type Release = () => PromiseOrDirect<void>;

website/src/pages/errors/peh.md

@@ -0,0 +1,43 @@
+# Pool error handling
+
+You're likely here because you received an error such as:
+
+```
+Your pool doesn't have error handlers! See: https://err.red/wpeh
+```
+
+This means you've passed your own
+[pg.Pool instance](https://node-postgres.com/apis/pool) to Graphile Worker, but
+that pool did not have error handling installed.
+
+If an error occurs on the pool and there is no event handler installed Node
+would exit
+[as described in the Node.js docs](https://nodejs.org/api/events.html#error-events).
+You get a network interruption, and your Worker might crash!
+
+Don't worry, we've installed error handlers for you, but that's not ideal - you
+should be handling this yourself. To do so, use code like this:
+
+```ts
+// Handle errors on the pool directly
+pgPool.on("error", (err) => {
+  console.error(`PostgreSQL idle client generated error: ${err.message}`);
+});
+// Handle errors on the client when it's checked out of the pool but isn't
+// actively being used
+pgPool.on("connect", (client) => {
+  client.on("error", (err) => {
+    console.error(`PostgreSQL active client generated error: ${err.message}`);
+  });
+});
+```
+
+Your code just needs to make sure the 'error' events on pool and client have
+handlers installed; the handlers don't actually have to _do_ anything - they
+could be NO-OPs.
+
+Typically these kinds of errors would occur when e.g. the connection between
+Node.js and PostgreSQL is interrupted (including when the PostgreSQL server
+shuts down). In these cases since the client is likely not actively being
+`await`-ed the error will have no handler, and will result in a process exit if
+unhandled.