wip: checkpoint

Back to you, @starfy84!

Co-authored-by: Dereck Tu <[email protected]>

Author: ericcrosson-bitgo

packages/typed-express-router/src/errors.ts

@@ -2,14 +2,16 @@ import express from 'express';
 import { Errors } from 'io-ts';
 import * as PathReporter from 'io-ts/lib/PathReporter';
 
+import { OnDecodeErrorFn } from './types';
+
 export function defaultOnDecodeError(
   errs: Errors,
   _req: express.Request,
-  res: express.Response,
-) {
+  _res: express.Response,
+): ReturnType<OnDecodeErrorFn> {
   const validationErrors = PathReporter.failure(errs);
   const validationErrorMessage = validationErrors.join('\n');
-  res.status(400).json({ error: validationErrorMessage }).end();
+  return { response: validationErrorMessage, statusCode: 400 };
 }
 
 export function defaultOnEncodeError(

packages/typed-express-router/src/index.ts

@@ -116,7 +116,7 @@ export function wrapRouter<Spec extends ApiSpec>(
           status: keyof (typeof route)['response'],
           payload: unknown,
         ) => {
-          const span = createSendEncodedSpan({
+          const encodeSpan = createSendEncodedSpan({
             apiName,
             httpRoute: route,
           });
@@ -129,7 +129,7 @@ export function wrapRouter<Spec extends ApiSpec>(
               typeof status === 'number'
                 ? status
                 : KeyToHttpStatus[status as keyof KeyToHttpStatus];
-            setSpanAttributes(span, {
+            setSpanAttributes(encodeSpan, {
               [ApiTsAttributes.API_TS_STATUS_CODE]: statusCode,
             });
             if (statusCode === undefined) {
@@ -146,28 +146,26 @@ export function wrapRouter<Spec extends ApiSpec>(
               res as WrappedResponse,
             );
           } catch (err) {
-            recordSpanEncodeError(span, err);
+            recordSpanEncodeError(encodeSpan, err);
             (options?.onEncodeError ?? onEncodeError)(
               err,
               req as WrappedRequest,
               res as WrappedResponse,
             );
           } finally {
-            endSpan(span);
+            endSpan(encodeSpan);
           }
         };
         next();
       };
 
-      const endDecodeSpanMiddleware: UncheckedRequestHandler = (req, _, next) => {
+      const endDecodeSpanMiddleware: UncheckedRequestHandler = (req, res, next) => {
         pipe(
           req.decoded,
-          E.matchW(
-            (errs) => {
-              recordSpanDecodeError(decodeSpan, errs);
-            },
-            () => {}, // No need to do anything on success
-          ),
+          E.getOrElseW((errs) => {
+            const error = (onDecodeError ?? onDecodeError)(errs, req, res);
+            recordSpanDecodeError(decodeSpan, error.response, error.statusCode);
+          }),
         );
         endSpan(decodeSpan);
         next();
@@ -202,7 +200,10 @@ export function wrapRouter<Spec extends ApiSpec>(
           req.decoded,
           E.matchW(
             (errs) => {
-              (options?.onDecodeError ?? onDecodeError)(errs, req, res);
+              const { response, statusCode } = (
+                options?.onDecodeError ?? onDecodeError
+              )(errs, req, res);
+              res.status(statusCode).json(response).end();
             },
             (value) => {
               req.decoded = value;

packages/typed-express-router/src/telemetry.ts

@@ -5,43 +5,42 @@
 // This module handles the optional dependency on @opentelemetry/api
 // It provides functionality for creating spans for decode and sendEncoded operations
 
-import { Errors } from 'io-ts';
-import type { SpanMetadata } from './types';
 import type { Attributes, Span, Tracer } from '@opentelemetry/api';
-import * as PathReporter from 'io-ts/lib/PathReporter';
+
+import type { Json, SpanMetadata } from './types';
 
 let otelApi: any;
 let tracer: Tracer | undefined;
 
-// Try to load @opentelemetry/api if available
+// Load @opentelemetry/api, if available.
 try {
   otelApi = require('@opentelemetry/api');
   if (otelApi) {
     tracer = otelApi.trace.getTracer('typed-express-router');
   }
 } catch (e) {
-  // Optional dependency not available, tracing will be disabled
+  // Optional dependency not available, so tracing will be disabled.
   tracer = undefined;
 }
 
 export const ApiTsAttributes = {
   /**
-   * Route name in the API-TS spec
+   * The Operation ID of the HTTP request
    */
-  API_TS_ROUTE_NAME: 'api_ts.route.name',
+  API_TS_OPERATION_ID: 'api_ts.operation_id',
 
   /**
-   * HTTP method for the route
+   * The method of the HTTP request
    */
-  API_TS_ROUTE_METHOD: 'api_ts.route.method',
+  API_TS_METHOD: 'api_ts.method',
 
   /**
-   * Path for the route
+   * The path of the HTTP request
    */
-  API_TS_ROUTE_PATH: 'api_ts.route.path',
+  API_TS_PATH: 'api_ts.path',
 
   /**
-   * Request status code
+   * Returned HTTP request status code
    */
   API_TS_STATUS_CODE: 'api_ts.status_code',
 };
@@ -56,15 +55,15 @@ export function createDefaultDecodeAttributes(metadata: SpanMetadata): Attribute
   const attributes: Attributes = {};
 
   if (metadata.apiName) {
-    attributes[ApiTsAttributes.API_TS_ROUTE_NAME] = metadata.apiName;
+    attributes[ApiTsAttributes.API_TS_OPERATION_ID] = metadata.apiName;
   }
 
   if (metadata.httpRoute) {
     if (metadata.httpRoute.method) {
-      attributes[ApiTsAttributes.API_TS_ROUTE_METHOD] = metadata.httpRoute.method;
+      attributes[ApiTsAttributes.API_TS_METHOD] = metadata.httpRoute.method;
     }
     if (metadata.httpRoute.path) {
-      attributes[ApiTsAttributes.API_TS_ROUTE_PATH] = metadata.httpRoute.path;
+      attributes[ApiTsAttributes.API_TS_PATH] = metadata.httpRoute.path;
     }
   }
 
@@ -81,15 +80,15 @@ export function createDefaultEncodeAttributes(metadata: SpanMetadata): Attribute
   const attributes: Attributes = {};
 
   if (metadata.apiName) {
-    attributes[ApiTsAttributes.API_TS_ROUTE_NAME] = metadata.apiName;
+    attributes[ApiTsAttributes.API_TS_OPERATION_ID] = metadata.apiName;
   }
 
   if (metadata.httpRoute) {
     if (metadata.httpRoute.method) {
-      attributes[ApiTsAttributes.API_TS_ROUTE_METHOD] = metadata.httpRoute.method;
+      attributes[ApiTsAttributes.API_TS_METHOD] = metadata.httpRoute.method;
     }
     if (metadata.httpRoute.path) {
-      attributes[ApiTsAttributes.API_TS_ROUTE_PATH] = metadata.httpRoute.path;
+      attributes[ApiTsAttributes.API_TS_PATH] = metadata.httpRoute.path;
     }
   }
 
@@ -148,22 +147,28 @@ export function recordSpanEncodeError(span: Span | undefined, error: unknown): v
 /**
  * Records errors on a decode span
  * @param span The span to record the errors on
- * @param errors The errors to record
+ * @param error The JSON error value to record
+ * @param statusCode The HTTP status code
  */
-export function recordSpanDecodeError(span: Span | undefined, errors: Errors): void {
+export function recordSpanDecodeError(
+  span: Span | undefined,
+  error: Json,
+  statusCode: number,
+): void {
   if (!span || !otelApi) {
     return;
   }
-  const validationErrors = PathReporter.failure(errors);
-  const validationErrorMessage = validationErrors.join('\n');
-  span.recordException(new Error(validationErrorMessage));
+  setSpanAttributes(span, {
+    [ApiTsAttributes.API_TS_STATUS_CODE]: statusCode,
+  });
+  span.recordException(JSON.stringify(error, null, 2));
   span.setStatus({ code: otelApi.SpanStatusCode.ERROR });
 }
 
 /**
  * Sets a span's attributes if it exists
  * @param span The span to modify
- * @param errors The attributes to modify the span with
+ * @param attributes The attributes to modify the span with
  */
 export function setSpanAttributes(
   span: Span | undefined,

packages/typed-express-router/src/types.ts

@@ -2,6 +2,12 @@ import { ApiSpec, HttpRoute, Method as HttpMethod } from '@api-ts/io-ts-http';
 import express from 'express';
 import * as t from 'io-ts';
 
+export type Json = boolean | number | string | null | JsonArray | JsonRecord;
+export interface JsonRecord {
+  readonly [key: string]: Json;
+}
+export interface JsonArray extends ReadonlyArray<Json> {}
+
 export type Methods = Lowercase<HttpMethod>;
 
 export type WrappedRequest<Decoded = unknown> = express.Request & {
@@ -22,7 +28,7 @@ export type OnDecodeErrorFn = (
   errs: t.Errors,
   req: express.Request,
   res: express.Response,
-) => void;
+) => { response: Json; statusCode: number };
 
 export type OnEncodeErrorFn = (
   err: unknown,

packages/typed-express-router/test/server.test.ts

@@ -361,8 +361,8 @@ test('should return a 400 when request fails to decode', async () => {
 
 test('should invoke custom decode error function', async () => {
   const router = createRouter(TestApiSpec, {
-    onDecodeError: (_errs, _req, res) => {
-      res.status(400).json('Custom decode error').end();
+    onDecodeError: (_errs, _req, _res) => {
+      return { response: 'Custom decode error', statusCode: 400 };
     },
   });
 
@@ -390,8 +390,8 @@ test('should invoke custom decode error function', async () => {
 
 test('should invoke per-route custom decode error function', async () => {
   const router = createRouter(TestApiSpec, {
-    onDecodeError: (_errs, _req, res) => {
-      res.status(400).json('Top-level decode error').end();
+    onDecodeError: (_errs, _req, _res) => {
+      return { response: 'Top-level decode error', statusCode: 400 };
     },
   });
 
@@ -403,8 +403,8 @@ test('should invoke per-route custom decode error function', async () => {
       },
     ],
     {
-      onDecodeError: (_errs, _req, res) => {
-        res.status(400).json('Route decode error').end();
+      onDecodeError: (_errs, _req, _res) => {
+        return { response: 'Route decode error', statusCode: 400 };
       },
       routeAliases: ['/helloNoPathParams'],
     },