Improve error handling of errors thrown from functions (#5)

Author: daniel-chambers

CHANGELOG.md

@@ -6,6 +6,9 @@ Changes to be included in the next upcoming release
 
 - Add support for JSDoc descriptions from object types ([#3](https://github.com/hasura/ndc-nodejs-lambda/pull/3))
 - Fix type name conflicts when using generic interfaces ([#4](https://github.com/hasura/ndc-nodejs-lambda/pull/4))
+- Improve error handling of errors thrown from functions ([#5](https://github.com/hasura/ndc-nodejs-lambda/pull/5))
+  - The entire causal stack trace is now captured as an error detail for unhandled errors
+  - `sdk.Forbidden`, `sdk.Conflict`, `sdk.UnprocessableContent` can be thrown to return error details to GraphQL API clients
 
 ## v0.11.0
 - Add support for parallel execution of readonly functions ([#2](https://github.com/hasura/ndc-nodejs-lambda/pull/2))

README.md

@@ -147,6 +147,68 @@ These types are unsupported as function parameter types or return types for func
 * [`void`](https://www.typescriptlang.org/docs/handbook/2/functions.html#void), [`object`](https://www.typescriptlang.org/docs/handbook/2/functions.html#object), [`unknown`](https://www.typescriptlang.org/docs/handbook/2/functions.html#unknown), [`never`](https://www.typescriptlang.org/docs/handbook/2/functions.html#never), [`any`](https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#any) types - to accept and return arbitrary JSON, use `sdk.JSONValue` instead
 * `null` and `undefined` - unless used in a union with a single other type
 
+### Error handling
+By default, unhandled errors thrown from functions are caught by the Lambda SDK host, and an `InternalServerError` is returned to Hasura. The details of the uncaught error (the message and stack trace) is captured and will be logged in the OpenTelemetry trace associated with the GraphQL request. However, the GraphQL API caller will receive a generic "internal error" response to their query. This is to ensure internal error details are not leaked to GraphQL API clients.
+
+If you want to return specific error details to the GraphQL API client, you can deliberately throw one of the below error classes (these error types correspond with the error status codes in the [NDC Specification](https://hasura.github.io/ndc-spec/specification/error-handling.html)):
+
+| Error Class              | Used When |
+|--------------------------|-----------|
+| sdk.Forbidden            | A permission check failed - for example, a mutation might fail because a check constraint was not met. |
+| sdk.Conflict             | A conflicting state would be created for the data source - for example, a mutation might fail because a foreign key constraint was not met. |
+| sdk.UnprocessableContent | There was something semantically incorrect in the request. For example, an invalid value for a function argument was received. |
+
+```typescript
+import * as sdk from "@hasura/ndc-lambda-sdk"
+
+/** @readonly */
+export function divide(x: number, y: number): number {
+  if (y === 0) {
+    throw new sdk.UnprocessableContent("Cannot divide by zero", { myErrorMetadata: "stuff", x, y })
+  }
+  return x / y;
+}
+```
+
+The GraphQL API will return the error from the API looking similar to this:
+
+```json
+{
+  "data": null,
+  "errors": [
+    {
+      "message": "ndc: Cannot divide by zero",
+      "path": ["divide"],
+      "extensions": {
+        "details": {
+          "myErrorMetadata": "stuff",
+          "x": 10,
+          "y": 0
+        }
+      }
+    }
+  ]
+}
+```
+
+If you must include stack traces in the GraphQL API response, you can collect and add them to the error details yourself using a helper function (`sdk.getErrorDetails`). However, it is not recommended to expose stack traces to API end users. Instead, API administrators can look in the GraphQL API tracing to find the stack traces logged.
+
+```typescript
+import * as sdk from "@hasura/ndc-lambda-sdk"
+
+/** @readonly */
+export function getStrings(): Promise<string[]> {
+  try {
+    return await queryForStrings();
+  } catch (e) {
+    const details = e instanceof Error
+      ? sdk.getErrorDetails(e) // Returns { message: string, stack: string }
+      : {};
+    throw new sdk.UnprocessableContent("Something went wrong :/", details)
+  }
+}
+```
+
 ### Parallel execution
 If functions are involved remote relationships in your Hasura metadata, then they may be queried in a [batch-based fashion](https://hasura.github.io/ndc-spec/specification/queries/variables.html). In this situation, any async functions that are marked with the `@readonly` JSDoc tag may be executed in parallel. The default degree of parallelism per query request to the connector is 10, but you may customise this by using the `@paralleldegree` JSDoc tag on your function.
 

ndc-lambda-sdk/.mocharc.json

@@ -1,5 +1,5 @@
 {
-  "require": "ts-node/register",
+  "require": ["ts-node/register", "test/init.ts"],
   "extensions": ["ts", "tsx"],
   "spec": [
     "test/**/*.test.ts"

ndc-lambda-sdk/package-lock.json

@@ -43,8 +43,10 @@
   },
   "devDependencies": {
     "@types/chai": "^4.3.11",
+    "@types/chai-as-promised": "^7.1.8",
     "@types/mocha": "^10.0.6",
     "chai": "^4.3.7",
+    "chai-as-promised": "^7.1.1",
     "mocha": "^10.2.0",
     "node-emoji": "^2.1.3",
     "node-postgres": "^0.6.2"

ndc-lambda-sdk/package.json

@@ -1,3 +1,4 @@
+import { EOL } from "os";
 import * as sdk from "@hasura/ndc-sdk-typescript"
 import pLimit from "p-limit";
 import * as schema from "./schema"
@@ -157,8 +158,10 @@ async function invokeFunction(func: Function, preparedArgs: unknown[], functionN
     }
     return result;
   } catch (e) {
-    if (e instanceof Error) {
-      throw new sdk.InternalServerError(`Error encountered when invoking function '${functionName}'`, { message: e.message, stack: e.stack });
+    if (e instanceof sdk.ConnectorError) {
+      throw e;
+    } else if (e instanceof Error) {
+      throw new sdk.InternalServerError(`Error encountered when invoking function '${functionName}'`, getErrorDetails(e));
     } else if (typeof e === "string") {
       throw new sdk.InternalServerError(`Error encountered when invoking function '${functionName}'`, { message: e });
     } else {
@@ -167,6 +170,47 @@ async function invokeFunction(func: Function, preparedArgs: unknown[], functionN
   }
 }
 
+export type ErrorDetails = {
+  message: string
+  stack: string
+}
+
+export function getErrorDetails(error: Error): ErrorDetails {
+  return {
+    message: error.message,
+    stack: buildCausalStackTrace(error),
+  }
+}
+
+function buildCausalStackTrace(error: Error): string {
+  let seenErrs: Error[] = [];
+  let currErr: Error | undefined = error;
+  let stackTrace = "";
+
+  while (currErr) {
+    seenErrs.push(currErr);
+
+    if (currErr.stack) {
+      stackTrace += `${currErr.stack}${EOL}`;
+    } else {
+      stackTrace += `${currErr.toString()}${EOL}`;
+    }
+    if (currErr.cause instanceof Error) {
+      if (seenErrs.includes(currErr.cause)) {
+        stackTrace += "<circular error cause loop>"
+        currErr = undefined;
+      } else {
+        stackTrace += `caused by `;
+        currErr = currErr.cause;
+      }
+
+    } else {
+      currErr = undefined;
+    }
+  }
+  return stackTrace;
+}
+
 export function reshapeResultToNdcResponseValue(value: unknown, type: schema.TypeDefinition, valuePath: string[], fields: Record<string, sdk.Field> | "AllColumns", objectTypes: schema.ObjectTypeDefinitions): unknown {
   switch (type.type) {
     case "array":

ndc-lambda-sdk/src/execution.ts

@@ -1,3 +1,3 @@
-import { JSONValue } from "./schema";
-
-export { JSONValue };
+export { JSONValue } from "./schema";
+export { ConnectorError, BadRequest, Forbidden, Conflict, UnprocessableContent, InternalServerError, NotSupported, BadGateway } from "@hasura/ndc-sdk-typescript";
+export { ErrorDetails, getErrorDetails } from "./execution";

ndc-lambda-sdk/src/sdk.ts

@@ -1,5 +1,5 @@
 import { describe, it } from "mocha";
-import { assert } from "chai";
+import { assert, expect } from "chai";
 import * as sdk from "@hasura/ndc-sdk-typescript"
 import { executeQuery } from "../../src/execution";
 import { FunctionNdcKind, FunctionsSchema } from "../../src/schema";
@@ -271,4 +271,97 @@ describe("execute query", function() {
     ]);
     assert.deepStrictEqual(functionCallCompletions, ["first", "third", "fourth", "second"]);
   });
+
+  describe("function error handling", function() {
+    const functionSchema: FunctionsSchema = {
+      functions: {
+        "theFunction": {
+          ndcKind: FunctionNdcKind.Function,
+          description: null,
+          parallelDegree: null,
+          arguments: [],
+          resultType: {
+            type: "named",
+            kind: "scalar",
+            name: "String"
+          }
+        }
+      },
+      objectTypes: {},
+      scalarTypes: {
+        "String": {}
+      }
+    };
+    const queryRequest: sdk.QueryRequest = {
+      collection: "theFunction",
+      query: {
+        fields: {},
+      },
+      arguments: {
+        "param": {
+          type: "literal",
+          value: "test"
+        }
+      },
+      collection_relationships: {}
+    };
+
+    it("Error -> sdk.InternalServerError", async function() {
+      const runtimeFunctions = {
+        "theFunction": () => {
+          throw new Error("BOOM!");
+        }
+      };
+
+      await expect(executeQuery(queryRequest, functionSchema, runtimeFunctions))
+        .to.be.rejectedWith(sdk.InternalServerError, "Error encountered when invoking function 'theFunction'")
+        .which.eventually.has.property("details")
+        .which.include.keys("stack")
+        .and.has.property("message", "BOOM!");
+    });
+
+    it("string -> sdk.InternalServerError", async function() {
+      const runtimeFunctions = {
+        "theFunction": () => {
+          throw "A bad way to throw errors";
+        }
+      };
+
+      await expect(executeQuery(queryRequest, functionSchema, runtimeFunctions))
+        .to.be.rejectedWith(sdk.InternalServerError, "Error encountered when invoking function 'theFunction'")
+        .which.eventually.has.property("details")
+        .and.has.property("message", "A bad way to throw errors");
+    });
+
+    it("unknown -> sdk.InternalServerError", async function() {
+      const runtimeFunctions = {
+        "theFunction": () => {
+          throw 666; // What are you even doing? 👊
+        }
+      };
+
+      await expect(executeQuery(queryRequest, functionSchema, runtimeFunctions))
+        .to.be.rejectedWith(sdk.InternalServerError, "Error encountered when invoking function 'theFunction'");
+    });
+
+    describe("sdk exceptions are passed through", function() {
+      const exceptions = [
+        sdk.BadRequest, sdk.Forbidden, sdk.Conflict, sdk.UnprocessableContent, sdk.InternalServerError, sdk.NotSupported, sdk.BadGateway
+      ];
+
+      for (const exceptionCtor of exceptions) {
+        it(`sdk.${exceptionCtor.name}`, async function() {
+          const runtimeFunctions = {
+            "theFunction": () => {
+              throw new exceptionCtor("Nope!", { deets: "stuff" });
+            }
+          };
+
+          await expect(executeQuery(queryRequest, functionSchema, runtimeFunctions))
+            .to.be.rejectedWith(exceptionCtor, "Nope!")
+            .and.eventually.property("details").deep.equals({"deets": "stuff"});
+        });
+      }
+    });
+  })
 });

ndc-lambda-sdk/test/execution/execute-query.test.ts

@@ -0,0 +1,8 @@
+// This file is hooked into mocha via .mocharc.json
+
+import chai from "chai";
+import chaiAsPromised from "chai-as-promised"
+
+export const mochaGlobalSetup = function() {
+  chai.use(chaiAsPromised);
+};