Merge pull request #97 from embed-team/fix/repo-cleanup

fix: harden embed failure handling

Author: rosethornbush

.moon/tasks/all.yml

@@ -9,6 +9,13 @@ tasks:
     args: "--check"
   lint:
     command: "oxlint --no-error-on-unmatched-pattern"
+    inputs:
+      - "**/*"
+      - "/packages/config/oxlint.config.ts"
+      - "/packages/config/oxlint/anti-slop/**/*"
+      - "/packages/config/package.json"
+      - "/pnpm-workspace.yaml"
+      - "/pnpm-lock.yaml"
   lint/fix:
     extends: "lint"
     args: "--fix"

apps/api/src/index.ts

@@ -5,6 +5,7 @@ import {
   formatLog,
   getErrorContext,
   getRequestId,
+  type LogContext,
 } from "@embedly/logging";
 import { Platforms } from "@embedly/platforms";
 import { httpInstrumentationMiddleware } from "@hono/otel";
@@ -21,6 +22,21 @@ import { version } from "../package.json";
 
 type ScrapeResponse = Awaited<ReturnType<(typeof Platforms)[keyof typeof Platforms]["transform"]>>;
 
+interface ApiLogContext extends LogContext {
+  request_id: string;
+  trace_id?: string;
+  span_id?: string;
+  source: string;
+  platform: string;
+  post_id: string;
+  force: boolean;
+  cache_status: "skipped" | "miss" | "hit" | "read_error" | "stored" | "write_error";
+  outcome: "success" | "error";
+  status_code: number;
+  error_type?: string;
+  duration_ms?: number;
+}
+
 const config: ResolveConfigFn<CloudflareBindings> = (env) => {
   if (!env.OTEL_ENDPOINT) throw new Error("OTEL_ENDPOINT is required.");
 
@@ -52,7 +68,7 @@ const app = new Hono<{ Bindings: CloudflareBindings }>()
       const { id, platform, force } = c.req.valid("json");
       const requestId = getRequestId(c.req.raw);
       const spanContext = trace.getActiveSpan()?.spanContext();
-      const logContext: Record<string, unknown> = {
+      const logContext: ApiLogContext = {
         request_id: requestId,
         trace_id: spanContext?.traceId,
         span_id: spanContext?.spanId,
@@ -73,25 +89,20 @@ const app = new Hono<{ Bindings: CloudflareBindings }>()
             const cachedItem = await cache.get<ScrapeResponse>(cacheKey, "json");
             if (cachedItem) {
               logContext.cache_status = "hit";
-              return c.json(cachedItem as ScrapeResponse, 200);
+              return c.json(cachedItem, 200);
             }
           } catch (cause) {
-            const problem = createProblem(EmbedlyErrors.CacheReadFailed, {
-              request_id: requestId,
-              context: { ...logContext, ...getErrorContext(cause) },
-            });
-            Object.assign(logContext, getErrorContext(cause), {
-              outcome: "error",
-              status_code: problem.status,
-              error_type: problem.type,
-            });
-            return c.json(problem, problem.status);
+            logContext.cache_status = "read_error";
+            console.warn(
+              formatLog("warn", EmbedlyErrors.CacheReadFailed, {
+                ...logContext,
+                ...getErrorContext(cause),
+              }),
+            );
           }
         }
 
-        // oxlint-disable-next-line import/namespace
-        const p = Platforms[platform as keyof typeof Platforms];
-        if (!p) {
+        if (!Object.hasOwn(Platforms, platform)) {
           const problem = createProblem(EmbedlyErrors.NoMatchesFound, {
             request_id: requestId,
             context: logContext,
@@ -104,6 +115,8 @@ const app = new Hono<{ Bindings: CloudflareBindings }>()
           });
           return c.json(problem, problem.status);
         }
+        // oxlint-disable-next-line import/namespace -- SAFETY: Object.hasOwn verified this registry key.
+        const p = Platforms[platform as keyof typeof Platforms];
 
         let raw: unknown;
         try {
@@ -125,6 +138,7 @@ const app = new Hono<{ Bindings: CloudflareBindings }>()
 
         let data: ScrapeResponse;
         try {
+          // SAFETY: raw came from this platform's fetch implementation.
           data = await p.transform(raw as any);
         } catch (cause) {
           const problem = createProblem(EmbedlyErrors.PlatformTransformFailed, {
@@ -145,16 +159,13 @@ const app = new Hono<{ Bindings: CloudflareBindings }>()
           });
           logContext.cache_status = "stored";
         } catch (cause) {
-          const problem = createProblem(EmbedlyErrors.CacheWriteFailed, {
-            request_id: requestId,
-            context: { ...logContext, ...getErrorContext(cause) },
-          });
-          Object.assign(logContext, getErrorContext(cause), {
-            outcome: "error",
-            status_code: problem.status,
-            error_type: problem.type,
-          });
-          return c.json(problem, problem.status);
+          logContext.cache_status = "write_error";
+          console.warn(
+            formatLog("warn", EmbedlyErrors.CacheWriteFailed, {
+              ...logContext,
+              ...getErrorContext(cause),
+            }),
+          );
         }
 
         return c.json(data, 200);

apps/bot/compose.yaml

@@ -11,7 +11,7 @@ services:
       timeout: 1s
       retries: 30
     ports:
-      - "6379:6379"
+      - "127.0.0.1:6379:6379"
 
   bot:
     container_name: embedly-bot

apps/bot/package.json

@@ -30,7 +30,6 @@
   "devDependencies": {
     "@embedly/config": "workspace:*",
     "@types/node": "catalog:",
-    "dotenv": "^17.4.2",
     "oxfmt": "catalog:",
     "oxlint": "catalog:",
     "tsdown": "^0.22.0",

apps/bot/src/commands/delete.ts

@@ -4,6 +4,7 @@ import {
   EmbedlyLogs,
   formatDiscordError,
   getErrorContext,
+  type LogContext,
 } from "@embedly/logging";
 import { Command } from "@sapphire/framework";
 import {
@@ -32,6 +33,25 @@ const DELETE_SUCCESS_MESSAGES = [
   "🧹 all tidy! embed removed as requested~",
 ];
 
+interface DeleteLogContext extends LogContext {
+  request_id: string;
+  trace_id?: string;
+  span_id?: string;
+  source: "context_menu";
+  interaction_id: string;
+  channel_id: string | null;
+  guild_id: string;
+  user_id: string;
+  message_id?: string;
+  original_author_id?: string;
+  has_manage_permission?: boolean;
+  outcome: "success" | "error";
+  status_code: number;
+  error_type?: string;
+  reason?: string;
+  duration_ms?: number;
+}
+
 export class DeleteCommand extends Command {
   public constructor(context: Command.LoaderContext, options: Command.Options) {
     super(context, {
@@ -62,7 +82,7 @@ export class DeleteCommand extends Command {
 
     const startedAt = Date.now();
     const requestId = `context_menu:${interaction.id}`;
-    const logContext: Record<string, unknown> = {
+    const logContext: DeleteLogContext = {
       request_id: requestId,
       source: "context_menu",
       interaction_id: interaction.id,
@@ -196,7 +216,6 @@ export class DeleteCommand extends Command {
 
           try {
             await msg.delete();
-            await this.container.messageCache.removeBotMessage(msg.id);
           } catch (error) {
             const problem = createProblem(EmbedlyErrors.DeleteFailed, {
               request_id: requestId,
@@ -216,6 +235,20 @@ export class DeleteCommand extends Command {
             return;
           }
 
+          try {
+            await this.container.messageCache.removeBotMessage(msg.id);
+          } catch (error) {
+            botErrors.add(1, {
+              ...metricContext,
+              error_type: EmbedlyErrors.MessageCacheFailed.type,
+            });
+            log("warn", EmbedlyErrors.MessageCacheFailed, {
+              ...logContext,
+              error_type: EmbedlyErrors.MessageCacheFailed.type,
+              ...getErrorContext(error),
+            });
+          }
+
           await interaction.editReply(
             DELETE_SUCCESS_MESSAGES[~~(DELETE_SUCCESS_MESSAGES.length * Math.random())],
           );