Improve Route Updates (#300)

* Increase delay before firing off probe

* Update routes on every incoming request

* Update package.json

* Modify sendRoutes to optionally use a promise store

* Add unit tests to client library (for routes helpers) and update sendRoutes

* Fix filter for running client library tests

* Add changelog entry

.github/workflows/build_frontends.yml

@@ -73,6 +73,9 @@ jobs:
       - name: Test Studio Frontend
         run: pnpm --filter @fiberplane/studio-frontend test
 
+      - name: Test Client Library
+        run: pnpm --filter @fiberplane/hono-otel test
+
       # Building
 
       - name: Build api, frontend, and client library

api/src/probe-routes.ts

@@ -40,8 +40,8 @@ export function startRouteProbeWatcher(watchDir: string) {
   const probeMaxRetries = 10;
   // Send the initial probe 500ms after startup
   const initialProbeDelay = 500;
-  // Add 1.5s delay for all successive probes (e.g., after filesystem change of watched project)
-  const probeDelay = 1500;
+  // Add 2.2s delay for all successive probes (e.g., after filesystem change of watched project)
+  const probeDelay = 2200;
 
   debouncedProbeRoutesWithExponentialBackoff(
     serviceTargetArgument,

packages/client-library-otel/package.json

@@ -4,7 +4,7 @@
   "author": "Fiberplane<[email protected]>",
   "type": "module",
   "main": "dist/index.js",
-  "version": "0.3.1-beta.1",
+  "version": "0.3.1-beta.2",
   "dependencies": {
     "@opentelemetry/api": "~1.9.0",
     "@opentelemetry/exporter-trace-otlp-http": "^0.52.1",
@@ -25,7 +25,8 @@
     "nodemon": "^3.1.7",
     "rimraf": "^6.0.1",
     "tsc-alias": "^1.8.10",
-    "typescript": "^5.4.5"
+    "typescript": "^5.4.5",
+    "vitest": "^1.6.0"
   },
   "publishConfig": {
     "access": "public"
@@ -50,6 +51,7 @@
     "dev": "wrangler dev sample/index.ts",
     "prepublishOnly": "pnpm run build",
     "clean": "rimraf dist",
+    "test": "vitest --run",
     "watch": "nodemon --watch src --ext ts,js,json --exec \"pnpm run build\""
   }
 }

packages/client-library-otel/src/instrumentation.ts

@@ -25,7 +25,11 @@ import {
 } from "./patch";
 import { PromiseStore } from "./promiseStore";
 import { propagateFpxTraceId } from "./propagation";
-import { isRouteInspectorRequest, respondWithRoutes } from "./routes";
+import {
+  isRouteInspectorRequest,
+  respondWithRoutes,
+  sendRoutes,
+} from "./routes";
 import type { HonoLikeApp, HonoLikeEnv, HonoLikeFetch } from "./types";
 import {
   getFromEnv,
@@ -125,10 +129,16 @@ export function instrument(app: HonoLikeApp, config?: FpxConfigOptions) {
             return await originalFetch(request, rawEnv, executionContext);
           }
 
-          // If the request is from the route inspector, respond with the routes
+          // If the request is from the route inspector, send latest routes to the Studio API and respond with 200 OK
           if (isRouteInspectorRequest(request)) {
             logger.debug("Responding to route inspector request");
-            return respondWithRoutes(webStandardFetch, endpoint, app);
+            const response = await respondWithRoutes(
+              webStandardFetch,
+              endpoint,
+              app,
+              logger,
+            );
+            return response;
           }
 
           const serviceName =
@@ -151,6 +161,14 @@ export function instrument(app: HonoLikeApp, config?: FpxConfigOptions) {
           });
 
           const promiseStore = new PromiseStore();
+
+          // NOTE - We want to report the latest routes to Studio on every request,
+          //        so that we have an up-to-date list of routes in the UI.
+          //        This will place the request in the promise store, so that we can
+          //        send the routes in the background while still ensuring the request
+          //        completes as usual.
+          sendRoutes(webStandardFetch, endpoint, app, logger, promiseStore);
+
           // Enable tracing for waitUntil
           const proxyExecutionCtx =
             executionContext && patchWaitUntil(executionContext, promiseStore);

packages/client-library-otel/src/routes.test.ts

@@ -0,0 +1,124 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { FpxLogger } from "./logger";
+import type { PromiseStore } from "./promiseStore";
+import {
+  isRouteInspectorRequest,
+  respondWithRoutes,
+  sendRoutes,
+} from "./routes";
+import type { HonoLikeApp, HonoLikeFetch } from "./types";
+
+describe("routes", () => {
+  describe("isRouteInspectorRequest", () => {
+    it("should return true when X-Fpx-Route-Inspector header is present", () => {
+      const request = new Request("https://example.com", {
+        headers: { "X-Fpx-Route-Inspector": "true" },
+      });
+      expect(isRouteInspectorRequest(request)).toBe(true);
+    });
+
+    it("should return false when X-Fpx-Route-Inspector header is not present", () => {
+      const request = new Request("https://example.com");
+      expect(isRouteInspectorRequest(request)).toBe(false);
+    });
+  });
+
+  describe("sendRoutes", () => {
+    let mockFetch: ReturnType<typeof vi.fn<[HonoLikeFetch], unknown>>;
+    let mockApp: HonoLikeApp;
+    let mockLogger: FpxLogger;
+    let mockPromiseStore: PromiseStore;
+
+    beforeEach(() => {
+      mockFetch = vi.fn().mockResolvedValue(new Response("OK"));
+      mockApp = {
+        routes: [{ method: "GET", path: "/test", handler: () => {} }],
+        fetch: vi.fn(),
+      } as unknown as HonoLikeApp;
+      mockLogger = { debug: vi.fn() } as unknown as FpxLogger;
+      mockPromiseStore = { add: vi.fn() } as unknown as PromiseStore;
+    });
+
+    it("should send routes successfully", async () => {
+      const result = await sendRoutes(
+        mockFetch as unknown as typeof fetch,
+        "https://fpx.example.com",
+        mockApp,
+        mockLogger,
+      );
+      expect(result).toBe(true);
+      expect(mockFetch).toHaveBeenCalledWith(
+        "https://fpx.example.com/v0/probed-routes",
+        expect.objectContaining({
+          method: "POST",
+          headers: { "Content-Type": "application/json" },
+          body: expect.any(String),
+        }),
+      );
+    });
+
+    it("should use promiseStore when provided", async () => {
+      await sendRoutes(
+        mockFetch as unknown as typeof fetch,
+        "https://fpx.example.com",
+        mockApp,
+        mockLogger,
+        mockPromiseStore,
+      );
+      expect(mockPromiseStore.add).toHaveBeenCalled();
+    });
+
+    it("should return false and log error on failure", async () => {
+      mockFetch.mockRejectedValue(new Error("Network error"));
+      const result = await sendRoutes(
+        mockFetch as unknown as typeof fetch,
+        "https://fpx.example.com",
+        mockApp,
+        mockLogger,
+      );
+      expect(result).toBe(false);
+      expect(mockLogger.debug).toHaveBeenCalledWith(
+        "Error sending routes to FPX:",
+        "Network error",
+      );
+    });
+  });
+
+  describe("respondWithRoutes", () => {
+    let mockFetch: ReturnType<typeof vi.fn<[HonoLikeFetch], unknown>>;
+    let mockApp: HonoLikeApp;
+    let mockLogger: FpxLogger;
+
+    beforeEach(() => {
+      mockFetch = vi.fn().mockResolvedValue(new Response("OK"));
+      mockApp = {
+        routes: [{ method: "GET", path: "/test", handler: () => {} }],
+        fetch: vi.fn(),
+      } as unknown as HonoLikeApp;
+      mockLogger = { debug: vi.fn() } as unknown as FpxLogger;
+    });
+
+    it("should respond with 200 OK when routes are sent successfully", async () => {
+      const response = await respondWithRoutes(
+        mockFetch as unknown as typeof fetch,
+        "https://fpx.example.com",
+        mockApp,
+        mockLogger,
+      );
+      expect(response.status).toBe(200);
+      expect(await response.text()).toBe("OK");
+    });
+
+    it("should respond with 500 error when sending routes fails", async () => {
+      mockFetch.mockRejectedValue(new Error("Network error"));
+      const response = await respondWithRoutes(
+        mockFetch as unknown as typeof fetch,
+        "https://fpx.example.com",
+        mockApp,
+        mockLogger,
+      );
+      expect(response.status).toBe(500);
+      expect(await response.text()).toBe("Error sending routes to FPX");
+    });
+  });
+});

packages/client-library-otel/src/routes.ts

@@ -1,3 +1,5 @@
+import type { FpxLogger } from "./logger";
+import type { PromiseStore } from "./promiseStore";
 import type { HonoLikeApp } from "./types";
 
 type FetchFn = typeof fetch;
@@ -12,44 +14,72 @@ export function isRouteInspectorRequest(request: Request) {
 }
 
 /**
- * Responds to the route inspector request by sending the routes to the FPX service.
+ * Sends the routes from the app to the FPX service.
  *
  * @param fetchFn - The fetch function to use to send the request.
  * @param fpxEndpoint - The endpoint of the FPX service.
  * @param app - The Hono app to get the routes from.
- * @returns
+ * @param logger - The logger to use to log messages.
+ * @param promiseStore - An optional promise store to add the request to.
+ * @returns `true` if the routes were sent successfully or if the request was added to the promise store, `false` otherwise
  */
-export function respondWithRoutes(
+export async function sendRoutes(
   fetchFn: FetchFn,
   fpxEndpoint: string,
   app: HonoLikeApp,
+  logger: FpxLogger,
+  promiseStore?: PromiseStore,
 ) {
   const routes = getRoutesFromApp(app) ?? [];
 
   try {
-    // HACK - Construct the routes endpoint here
-    //        We could also do what we did before and submit the routes to the same `/v1/traces`
-    //        but that route handler is so chaotic right now I wanted to have this as a separate
-    //        endpoint.
+    // NOTE - Construct url to the routes endpoint here, given the FPX endpoint.
+    //        The routes endpoint is what we use to update the list of registered routes in Studio.
     const routesEndpoint = getRoutesEndpoint(fpxEndpoint);
-    fetchFn(routesEndpoint.toString(), {
+    const responsePromise = fetchFn(routesEndpoint.toString(), {
       method: "POST",
       headers: {
         "Content-Type": "application/json",
       },
       body: JSON.stringify({ routes }),
-    }).catch((_e) => {
-      // NOTE - We're not awaiting this fetch, so we need to catch its errors
-      //        to avoid unhandled promise rejections.
-      // TODO - Use a logger, or only log if library debugging is enabled
-      // console.error("Error sending routes to FPX", e);
     });
-  } catch (_e) {
-    // TODO - Use a logger, or only log if library debugging is enabled
-    // console.error("Error sending routes to FPX", e);
+
+    if (promiseStore) {
+      promiseStore.add(responsePromise);
+    } else {
+      await responsePromise;
+    }
+    return true;
+  } catch (e) {
+    const message = e instanceof Error ? e.message : "Unknown error";
+    logger?.debug("Error sending routes to FPX:", message);
+    return false;
   }
+}
 
-  return new Response("OK");
+/**
+ * Sends the routes from the app to the FPX service, then returns a response that can be used
+ * for fpx route inspection requests.
+ *
+ * @param fetchFn - The fetch function to use to send the request.
+ * @param fpxEndpoint - The endpoint of the FPX service.
+ * @param app - The Hono app to get the routes from.
+ * @returns A Response that can be sent back to the client
+ */
+export async function respondWithRoutes(
+  fetchFn: FetchFn,
+  fpxEndpoint: string,
+  app: HonoLikeApp,
+  logger: FpxLogger,
+) {
+  try {
+    const success = await sendRoutes(fetchFn, fpxEndpoint, app, logger);
+    return new Response(success ? "OK" : "Error sending routes to FPX", {
+      status: success ? 200 : 500,
+    });
+  } catch (_e) {
+    return new Response("Error sending routes to FPX", { status: 500 });
+  }
 }
 
 function getRoutesFromApp(app: HonoLikeApp) {

packages/client-library-otel/tsconfig.json

@@ -6,7 +6,7 @@
     "strict": true,
     "skipLibCheck": true,
     "lib": ["ESNext"],
-    "types": ["@cloudflare/workers-types"],
+    "types": ["vitest/globals", "@cloudflare/workers-types"],
     "esModuleInterop": true,
     "outDir": "./dist",
     "rootDir": "./src",

packages/client-library-otel/vitest.config.ts

@@ -0,0 +1,7 @@
+import { defineConfig } from "vitest/config";
+
+export default defineConfig({
+  test: {
+    globals: true,
+  },
+});

www/src/content/changelog/!canary.mdx

@@ -10,6 +10,8 @@ draft: true
 
 - **Improved AI request generation** We've added some static analysis smarts for wrangler projects in order to give LLM providers more context about the code that powers each route.
 
+- **Improved Route Updating** We've added some improvements to how we refresh your list of API routes in the Studio.
+
 ### Bug fixes
 
 - **D1 autoinstrumentation** The client library, `@fiberplane/hono-otel`, now instruments D1 queries in latest versions of wrangler/miniflare.