Address comments

Author: mxiao-cll

src/adapter/basic.ts

@@ -468,20 +468,16 @@ export class Adapter<
       fallbackReq,
       replySent,
       endpoint.transportRoutes.get(fallback.transportName),
-    ).then(
-      (response) => ({ response }),
-      (error) => ({ error }),
     )
 
     try {
       return await this.handleSingleTransportRequest(req, replySent, transport)
     } catch (primaryError) {
-      const fallbackResult = await fallbackResponsePromise
-      if ('response' in fallbackResult) {
-        return fallbackResult.response
+      try {
+        return await fallbackResponsePromise
+      } catch (_fallbackError) {
+        throw primaryError
       }
-
-      throw primaryError
     }
   }
 

src/adapter/endpoint.ts

@@ -79,24 +79,7 @@ export class AdapterEndpoint<T extends EndpointGenerics> implements AdapterEndpo
     this.overrides = params.overrides
     this.requestTransforms = [this.symbolOverrider.bind(this), ...(params.requestTransforms || [])]
 
-    // Validate fallback structure if it's defined
-    Object.entries(this.fallbackTransport || {}).forEach(([primary, fallback]) => {
-      if (primary === fallback) {
-        throw new AdapterError({
-          statusCode: 400,
-          message: `Fallback transport "${fallback}" cannot be the same as primary transport.`,
-        })
-      }
-
-      if (!this.transportRoutes.get(fallback)) {
-        throw new AdapterError({
-          statusCode: 400,
-          message: `No fallback transport found for key "${fallback}", must be one of ${JSON.stringify(
-            this.transportRoutes.routeNames(),
-          )}`,
-        })
-      }
-    })
+    this.validateFallbackTransport()
   }
 
   /**
@@ -249,4 +232,40 @@ export class AdapterEndpoint<T extends EndpointGenerics> implements AdapterEndpo
     }
     return rawRequestBody.data?.transport
   }
+
+  private validateFallbackTransport() {
+    if (this.cacheKeyGenerator && this.fallbackTransport) {
+      throw new AdapterError({
+        message: 'fallbackTransport not allowed for endpoints with cacheKeyGenerator',
+        statusCode: 404,
+      })
+    }
+
+    Object.entries(this.fallbackTransport || {}).forEach(([primary, fallback]) => {
+      if (primary === fallback) {
+        throw new AdapterError({
+          statusCode: 400,
+          message: `Fallback transport "${fallback}" cannot be the same as primary transport.`,
+        })
+      }
+
+      if (!this.transportRoutes.get(primary)) {
+        throw new AdapterError({
+          statusCode: 400,
+          message: `No primary transport found for key "${primary}", must be one of ${JSON.stringify(
+            this.transportRoutes.routeNames(),
+          )}`,
+        })
+      }
+
+      if (!this.transportRoutes.get(fallback)) {
+        throw new AdapterError({
+          statusCode: 400,
+          message: `No fallback transport found for key "${fallback}", must be one of ${JSON.stringify(
+            this.transportRoutes.routeNames(),
+          )}`,
+        })
+      }
+    })
+  }
 }

src/util/types.ts

@@ -39,7 +39,6 @@ export type AdapterRequestContext<T> = {
   cacheKey: string
 
   /** Fallback transport context to use if the primary transport is unable to return data */
-  // TODO: Implement using this
   fallback?: {
     transportName: string
     cacheKey: string

src/validation/index.ts

@@ -106,20 +106,8 @@ export const validatorMiddleware: AdapterMiddlewareBuilder =
       req.requestContext = { ...req.requestContext, meta: { metrics } }
     }
 
-    const fallbackTransportName = endpoint.getFallbackTransportNameForRequest(
-      req.requestContext.transportName,
-      adapter.config.settings,
-    )
-
     // Now that all the transformations have been applied, all that's left is calculating the cache key
     if (endpoint.cacheKeyGenerator) {
-      if (fallbackTransportName) {
-        throw new AdapterInputError({
-          message: 'fallbackTransport not supported for endpoints with cacheKeyGenerator',
-          statusCode: 404,
-        })
-      }
-
       let cacheKey
       cacheKey = endpoint.cacheKeyGenerator(req.requestContext.data)
       if (cacheKey.length > adapter.config.settings.MAX_COMMON_KEY_SIZE) {
@@ -149,6 +137,10 @@ export const validatorMiddleware: AdapterMiddlewareBuilder =
         transportName: req.requestContext.transportName,
       })
 
+      const fallbackTransportName = endpoint.getFallbackTransportNameForRequest(
+        req.requestContext.transportName,
+        adapter.config.settings,
+      )
       if (fallbackTransportName) {
         req.requestContext.fallback = {
           transportName: fallbackTransportName,

test/cache/cache-key.test.ts

@@ -225,61 +225,6 @@ test.serial('builds fallback cache key from fallback transport name', async (t)
   )
 })
 
-test.serial('custom cache key generator throws on fallback transport', async (t) => {
-  const config = new AdapterConfig(
-    {},
-    {
-      envDefaultOverrides: {
-        TRANSPORT_FALLBACK_ENABLED: true,
-      },
-    },
-  )
-  const adapter = new Adapter({
-    name: 'TEST',
-    defaultEndpoint: 'test-custom-cache-key-fallback',
-    config,
-    endpoints: [
-      new AdapterEndpoint<NopTransportTypes>({
-        name: 'test-custom-cache-key-fallback',
-        cacheKeyGenerator: (_) => `test:custom_cache_key`,
-        transportRoutes: new TransportRoutes<NopTransportTypes>()
-          .register(
-            'primary',
-            new (class extends NopTransport {
-              override async foregroundExecute(
-                req: AdapterRequest<NopTransportTypes['Parameters']>,
-              ) {
-                return {
-                  data: null,
-                  statusCode: 200,
-                  result: (req.requestContext.fallback?.cacheKey ||
-                    req.requestContext.cacheKey) as unknown as null,
-                  timestamps: {
-                    providerDataRequestedUnixMs: 0,
-                    providerDataReceivedUnixMs: 0,
-                    providerIndicatedTimeUnixMs: undefined,
-                  },
-                }
-              }
-            })(),
-          )
-          .register('fallback', new NopTransport()),
-        defaultTransport: 'primary',
-        fallbackTransport: { primary: 'fallback' },
-      }),
-    ],
-  })
-  const testAdapter = await TestAdapter.start(adapter, t.context)
-
-  const response = await testAdapter.request({})
-
-  t.is(response.statusCode, 404)
-  t.is(
-    response.json().error.message,
-    'fallbackTransport not supported for endpoints with cacheKeyGenerator',
-  )
-})
-
 test.serial('custom cache key is truncated if over max size', async (t) => {
   const response = await t.context.testAdapter.request({
     endpoint: 'test-custom-cache-key-long',

test/transports/routing.test.ts

@@ -738,6 +738,22 @@ test.serial('fallback transport must be registered', async (t) => {
   )
 })
 
+test.serial("fallback's primary transport must be registered", async (t) => {
+  t.throws(
+    () =>
+      new AdapterEndpoint<BaseEndpointTypes>({
+        inputParameters,
+        name: 'price',
+        transportRoutes: transports,
+        fallbackTransport: { invalid: 'WEBSOCKET' },
+      }),
+    {
+      message:
+        'No primary transport found for key "invalid", must be one of ["websocket","batch","sse"]',
+    },
+  )
+})
+
 test.serial('fallback transport cannot match primary transport', async (t) => {
   t.throws(
     () =>
@@ -753,6 +769,22 @@ test.serial('fallback transport cannot match primary transport', async (t) => {
   )
 })
 
+test.serial('fallback transport not allowed with cacheKeyGenerator', async (t) => {
+  t.throws(
+    () =>
+      new AdapterEndpoint<BaseEndpointTypes>({
+        inputParameters,
+        name: 'price',
+        transportRoutes: transports,
+        fallbackTransport: { batch: 'WEBSOCKET' },
+        cacheKeyGenerator: () => 'cache',
+      }),
+    {
+      message: 'fallbackTransport not allowed for endpoints with cacheKeyGenerator',
+    },
+  )
+})
+
 test.serial('transport override routes to correct Transport', async (t) => {
   axiosMock
     .onPost(`${restUrl}/price`, {