Feature/176 f 036 improve GitHub command ux and error handling

README.md

@@ -230,7 +230,8 @@ OmegaBot is online
 │   │   │   ├── commandMeta.ts
 │   │   │   ├── cooldowns.ts
 │   │   │   ├── fetchChannelMessages.ts
-│   │   │   └── interactionHandler.ts
+│   │   │   ├── interactionHandler.ts
+│   │   │   └── safeReply.ts
 │   │   ├── faq
 │   │   │   ├── _shared.ts
 │   │   │   ├── faqService.ts
@@ -243,6 +244,7 @@ OmegaBot is online
 │   │   ├── github
 │   │   │   ├── githubApi.ts
 │   │   │   ├── githubClient.ts
+│   │   │   ├── githubErrorMessage.ts
 │   │   │   ├── lastSeenStore.ts
 │   │   │   ├── prFormatter.ts
 │   │   │   ├── prPoller.ts

src/commands/fun/subcommands/chucknorris.ts

@@ -7,6 +7,11 @@ type ChuckNorrisApiResponse = {
   value: string;
 };
 
+type ChuckNorrisSearchResponse = {
+  total?: number;
+  result?: ChuckNorrisApiResponse[];
+};
+
 export type ChuckNorrisMode =
   | { kind: "random" }
   | { kind: "category"; category: string }
@@ -34,27 +39,80 @@ export async function run(
       "[fun/chucknorris] sent",
     );
   } catch (err) {
-    logger.error({ err, mode }, "[fun/chucknorris] failed");
-    await interaction.editReply(
-      "Chuck Norris is currently roundhouse kicking the API. Try again later.",
-    );
+    const userMessage = toUserMessage(err);
+
+    logger.error({ err, mode, userMessage }, "[fun/chucknorris] failed");
+    await interaction.editReply(userMessage);
+  }
+}
+
+class UserFacingError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "UserFacingError";
+  }
+}
+
+function toUserMessage(err: unknown): string {
+  if (err instanceof UserFacingError) return err.message;
+
+  // Keep the fun fallback, but also provide a tiny bit of “why” for non-user errors.
+  if (err instanceof Error) {
+    // Avoid leaking raw internals; give a short category of failure.
+    if (err.message.toLowerCase().includes("network")) {
+      return "I couldn’t reach the Chuck Norris API (network issue). Try again in a bit.";
+    }
+    if (err.message.toLowerCase().includes("api error")) {
+      return "The Chuck Norris API returned an error. Try again later.";
+    }
   }
+
+  return "Chuck Norris is currently roundhouse kicking the API. Try again later.";
 }
 
 async function fetchChuckNorris(mode: ChuckNorrisMode): Promise<string> {
   if (mode.kind === "random")
     return fetchSingle("https://api.chucknorris.io/jokes/random");
 
   if (mode.kind === "category") {
+    const category = mode.category.trim();
+    if (!category) {
+      throw new UserFacingError("Please provide a category.");
+    }
+
     const url = `https://api.chucknorris.io/jokes/random?category=${encodeURIComponent(
-      mode.category,
+      category,
     )}`;
-    return fetchSingle(url);
+
+    // fetchSingle throws on non-OK; we intercept 404 here to explain “why”.
+    try {
+      return await fetchSingle(url);
+    } catch (err) {
+      if (isHttpError(err, 404)) {
+        const categories = await safeFetchCategories();
+        const hint = categories?.length
+          ? `Valid categories include: ${categories
+              .slice(0, 12)
+              .map((c) => `\`${c}\``)
+              .join(", ")}`
+          : "Try something like `dev`, `movie`, or `science`.";
+
+        throw new UserFacingError(
+          `That category was not found: \`${category}\`. ${hint}`,
+        );
+      }
+      throw err;
+    }
   }
 
   // mode.kind === "search"
+  const query = mode.query.trim();
+  if (!query) {
+    throw new UserFacingError("Please provide a search query.");
+  }
+
   const url = `https://api.chucknorris.io/jokes/search?query=${encodeURIComponent(
-    mode.query,
+    query,
   )}`;
 
   const res = await fetch(url, {
@@ -65,29 +123,54 @@ async function fetchChuckNorris(mode: ChuckNorrisMode): Promise<string> {
   });
 
   if (!res.ok) {
+    // For search, a 400 usually means bad query; treat as user-facing
+    if (res.status === 400) {
+      throw new UserFacingError(
+        "That search query didn’t work. Try a simpler word or phrase.",
+      );
+    }
     throw new Error(`Chuck Norris API error: ${res.status} ${res.statusText}`);
   }
 
-  const data = (await res.json()) as { result?: ChuckNorrisApiResponse[] };
-  const first = data.result?.[0]?.value;
+  const data = (await res.json()) as ChuckNorrisSearchResponse;
+  const results = Array.isArray(data.result) ? data.result : [];
 
-  if (!first || typeof first !== "string") {
-    throw new Error("No results for that search");
+  if (!results.length) {
+    throw new UserFacingError(`No results for \`${query}\`. Try something broader.`);
   }
 
-  return first.trim();
+  // Pick a random result so it feels fresh
+  const pick = results[Math.floor(Math.random() * results.length)];
+  const joke = pick?.value;
+
+  if (!joke || typeof joke !== "string") {
+    throw new Error("Chuck Norris API returned an unexpected response shape");
+  }
+
+  return joke.trim();
 }
 
 async function fetchSingle(url: string): Promise<string> {
-  const res = await fetch(url, {
-    headers: {
-      Accept: "application/json",
-      "User-Agent": "OmegaBot",
-    },
-  });
+  let res: Response;
+
+  try {
+    res = await fetch(url, {
+      headers: {
+        Accept: "application/json",
+        "User-Agent": "OmegaBot",
+      },
+    });
+  } catch (err) {
+    throw new Error(
+      `Network error calling Chuck Norris API: ${(err as Error)?.message ?? String(err)}`,
+    );
+  }
 
   if (!res.ok) {
-    throw new Error(`Chuck Norris API error: ${res.status} ${res.statusText}`);
+    throw new HttpError(
+      `Chuck Norris API error: ${res.status} ${res.statusText}`,
+      res.status,
+    );
   }
 
   const data = (await res.json()) as ChuckNorrisApiResponse;
@@ -98,3 +181,40 @@ async function fetchSingle(url: string): Promise<string> {
 
   return data.value.trim();
 }
+
+class HttpError extends Error {
+  constructor(
+    message: string,
+    public status: number,
+  ) {
+    super(message);
+    this.name = "HttpError";
+  }
+}
+
+function isHttpError(err: unknown, status: number): boolean {
+  return err instanceof HttpError && err.status === status;
+}
+
+async function safeFetchCategories(): Promise<string[] | null> {
+  try {
+    const res = await fetch("https://api.chucknorris.io/jokes/categories", {
+      headers: {
+        Accept: "application/json",
+        "User-Agent": "OmegaBot",
+      },
+    });
+
+    if (!res.ok) return null;
+
+    const data = (await res.json()) as unknown;
+
+    if (!Array.isArray(data) || !data.every((x) => typeof x === "string")) {
+      return null;
+    }
+
+    return data as string[];
+  } catch {
+    return null;
+  }
+}

src/services/discord/cooldowns.ts

@@ -1,41 +1,58 @@
-// src/services/discord/cooldowns.ts
+// src/services/discord/safeReply.ts
+//
+// Safe reply helper for Discord interactions.
+//
+// Goal:
+// - Avoid "Interaction already replied" and "Unknown interaction" pitfalls
+// - Let callers write one consistent "respond" call
+//
+// Behavior:
+// - If interaction already has a reply or was deferred, use editReply(string)
+// - Otherwise use reply({ content, flags? })
+//
+// Notes:
+// - Discord does not allow setting Ephemeral on editReply.
+//   So we only apply ephemeral flags on the initial reply.
 
-/**
- * In-memory cooldown store.
- *
- * Key format: `${userId}:${commandName}`
- * Value: timestamp (ms) when the command can be used again
- */
-const cooldowns = new Map<string, number>();
+import { MessageFlags, type InteractionReplyOptions } from "discord.js";
+
+export type SafeReplyOptions = {
+  content: string;
+  ephemeral?: boolean;
+};
 
-function makeKey(userId: string, commandName: string): string {
-  return `${userId}:${commandName}`;
+function toReplyOptions(opts: SafeReplyOptions): InteractionReplyOptions {
+  if (opts.ephemeral) {
+    return { content: opts.content, flags: MessageFlags.Ephemeral };
+  }
+  return { content: opts.content };
 }
 
 /**
- * Returns remaining cooldown time in milliseconds.
- * Returns 0 if no cooldown is active.
+ * The minimal shape we need from an interaction.
+ *
+ * Important:
+ * - editReply only accepts a string (or edit options), and cannot set Ephemeral.
+ * - We only ever edit with a string, so keep the type strict to avoid mismatches.
  */
-export function getCooldownRemainingMs(userId: string, commandName: string): number {
-  const key = makeKey(userId, commandName);
-  const expiresAt = cooldowns.get(key);
-
-  if (!expiresAt) return 0;
-
-  const remaining = expiresAt - Date.now();
-  return remaining > 0 ? remaining : 0;
-}
+type SafeInteraction = {
+  replied: boolean;
+  deferred: boolean;
+  reply: (options: InteractionReplyOptions) => Promise<unknown>;
+  editReply: (content: string) => Promise<unknown>;
+};
 
 /**
- * Sets a cooldown for a user + command.
+ * Safely respond to an interaction exactly once.
  */
-export function setCooldown(
-  userId: string,
-  commandName: string,
-  durationMs: number,
-): void {
-  if (durationMs <= 0) return;
+export async function safeReply(
+  interaction: SafeInteraction,
+  opts: SafeReplyOptions,
+): Promise<void> {
+  if (interaction.replied || interaction.deferred) {
+    await interaction.editReply(opts.content);
+    return;
+  }
 
-  const key = makeKey(userId, commandName);
-  cooldowns.set(key, Date.now() + durationMs);
+  await interaction.reply(toReplyOptions(opts));
 }

src/services/discord/interactionHandler.ts

@@ -17,23 +17,7 @@
 import type { Interaction, ChatInputCommandInteraction } from "discord.js";
 import type { CommandClient } from "./commandLoader.js";
 import { logger } from "../../utils/logger.js";
-import { getCooldownRemainingMs, setCooldown } from "./cooldowns.js";
-
-/**
- * Per-command cooldown durations (ms).
- *
- * Keep this list small and focused:
- * - Only expensive commands should be throttled (LLM calls, external APIs).
- * - Defaults to 0 (no cooldown) for commands not listed here.
- *
- * Adjust as needed.
- */
-const COOLDOWN_MS: Record<string, number> = {
-  summary: 30_000,
-  // github lookups (example)
-  // gh: 5_000,
-  // pr: 5_000,
-};
+import { safeReply } from "./safeReply.js";
 
 /**
  * Handle a single Discord interaction.
@@ -83,38 +67,13 @@ export async function handleInteraction(
       "Received interaction for unknown command",
     );
 
-    await interaction.reply({
+    await safeReply(interaction, {
       content: "Command not found.",
       ephemeral: true,
     });
     return;
   }
 
-  /**
-   * Cooldowns (per-user, per-command)
-   *
-   * Cooldown is enforced here so every command benefits consistently.
-   * We set cooldown BEFORE execution to prevent spam even if the command fails.
-   */
-  const userId = interaction.user.id;
-  const commandName = interaction.commandName;
-
-  const remainingMs = getCooldownRemainingMs(userId, commandName);
-  if (remainingMs > 0) {
-    const seconds = Math.ceil(remainingMs / 1000);
-
-    await interaction.reply({
-      content: `⏳ That command is on cooldown. Try again in ${seconds}s.`,
-      ephemeral: true,
-    });
-    return;
-  }
-
-  const durationMs = COOLDOWN_MS[commandName] ?? 0;
-  if (durationMs > 0) {
-    setCooldown(userId, commandName, durationMs);
-  }
-
   /**
    * Execute the command safely.
    *
@@ -137,12 +96,9 @@ export async function handleInteraction(
       "Slash command execution failed",
     );
 
-    const message = "Something went wrong while running this command.";
-
-    if (interaction.replied || interaction.deferred) {
-      await interaction.editReply(message);
-    } else {
-      await interaction.reply({ content: message, ephemeral: true });
-    }
+    await safeReply(interaction, {
+      content: "Something went wrong while running this command.",
+      ephemeral: true,
+    });
   }
 }