From fd99c5884b5c67643ae98a39bcc744cb482b126f Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Tue, 18 Aug 2026 09:01:24 -0400 Subject: [PATCH 1/3] fix(deps): reorder category risk order, fix maintenance fallback Adds "digest" as a distinct DepCategory (previously folded into "patch") and reorders CATEGORY_SORT_ORDER to maintenance -> pin -> digest -> patch -> minor -> major -> other, matching Renovate's documented digest/patch/ minor/major ordering (renovatebot/renovate#10217). Fixes depCategory()'s fallback for unparseable PRs, which was defaulting to "maintenance" (verified via a real production PR) instead of "other" - silently sorting unknown-risk PRs into the safest slot. Also closes a related gap in the label-based fallback, which checked major/minor/patch labels but never digest/pin/maintenance. Extends DependencyFiltersSchema's updateType enum to include "digest" before shipping the new filter option, avoiding a localStorage schema mismatch that would otherwise wipe all view state on next load. --- docs/USER_GUIDE.md | 4 +- .../components/dashboard/DependenciesTab.tsx | 25 ++--- src/app/stores/view.ts | 2 +- .../dashboard/DependenciesTab.test.tsx | 94 +++++++++++++++++++ tests/stores/view.test.ts | 5 + 5 files changed, 115 insertions(+), 15 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 5142eda2..23eb9ec5 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -301,7 +301,7 @@ Unlike the Pull Requests tab (which groups by repo), the Dependencies tab groups | **Needs Action** | CI pending, checks still running, or PR is a draft — not yet actionable | | **Stale** | PR has been open more than 14 days without merging — may need a rebase or manual review | -Within each group, PRs are sorted by repository name, then update category (maintenance, pin, patch, minor, major), then update date. +Within each group, PRs are sorted by repository name, then update category (maintenance, pin, digest, patch, minor, major), then update date. ### Abandoned Dependencies @@ -323,7 +323,7 @@ Go to **Settings > Dependencies** to configure: | Filter | Options | Default | |--------|---------|---------| -| Update type | All / Major / Minor / Patch | All | +| Update type | All / Major / Minor / Patch / Digest | All | | Bot | All / (detected bot logins) | All (shown when multiple bots are active) | The update type filter reads the PR title for SemVer version bump signals (e.g., `1.x → 2.x` = Major). PRs with titles that do not contain recognizable version patterns are grouped under the currently active filter if it is set to All. diff --git a/src/app/components/dashboard/DependenciesTab.tsx b/src/app/components/dashboard/DependenciesTab.tsx index b9cc2591..00158b3a 100644 --- a/src/app/components/dashboard/DependenciesTab.tsx +++ b/src/app/components/dashboard/DependenciesTab.tsx @@ -34,6 +34,7 @@ const UPDATE_TYPE_OPTIONS: FilterChipGroupDef = { options: [ { value: "maintenance", label: "Maintenance" }, { value: "pin", label: "Pin" }, + { value: "digest", label: "Digest" }, { value: "patch", label: "Patch" }, { value: "minor", label: "Minor" }, { value: "major", label: "Major" }, @@ -48,20 +49,16 @@ const STATUS_META: Record = { "stale": { label: "Stale" }, }; -type DepCategory = "major" | "minor" | "patch" | "pin" | "maintenance" | "other"; - -function mapUpdateType(ut: NonNullable): DepCategory { - if (ut === "digest") return "patch"; - return ut; -} +type DepCategory = "major" | "minor" | "patch" | "digest" | "pin" | "maintenance" | "other"; const CATEGORY_SORT_ORDER: Record = { maintenance: 0, pin: 1, - patch: 2, - minor: 3, - major: 4, - other: 5, + digest: 2, + patch: 3, + minor: 4, + major: 5, + other: 6, }; const CATEGORY_BADGE_CLASS: Partial> = { @@ -69,21 +66,25 @@ const CATEGORY_BADGE_CLASS: Partial> = { minor: "badge-warning", patch: "badge-success", pin: "badge-success", + digest: "badge-success", }; function depCategory(pr: PullRequest, versionInfo: VersionInfo | null): DepCategory { - if (versionInfo?.updateType) return mapUpdateType(versionInfo.updateType); + if (versionInfo?.updateType) return versionInfo.updateType; const titleLower = pr.title.toLowerCase(); if (/pin\s+dep/.test(titleLower)) return "pin"; if (/lock\s*file\s+maintenance/.test(titleLower)) return "maintenance"; - const fallback: DepCategory = versionInfo ? "other" : "maintenance"; + const fallback: DepCategory = "other"; for (const l of pr.labels) { const name = l.name.toLowerCase(); if (name === "major") return "major"; if (name === "minor") return "minor"; if (name === "patch") return "patch"; + if (name === "digest") return "digest"; + if (name === "pin") return "pin"; + if (name === "maintenance") return "maintenance"; } return fallback; } diff --git a/src/app/stores/view.ts b/src/app/stores/view.ts index b04cabf1..d35c7f7c 100644 --- a/src/app/stores/view.ts +++ b/src/app/stores/view.ts @@ -47,7 +47,7 @@ export const ActionsFiltersSchema = z.object({ }); export const DependencyFiltersSchema = z.object({ - updateType: z.enum(["all", "major", "minor", "patch", "pin", "maintenance", "other"]).default("all"), + updateType: z.enum(["all", "major", "minor", "patch", "pin", "digest", "maintenance", "other"]).default("all"), bot: z.string().default("all"), }); diff --git a/tests/components/dashboard/DependenciesTab.test.tsx b/tests/components/dashboard/DependenciesTab.test.tsx index ea47e4b8..b9b3df9a 100644 --- a/tests/components/dashboard/DependenciesTab.test.tsx +++ b/tests/components/dashboard/DependenciesTab.test.tsx @@ -467,6 +467,100 @@ describe("DependenciesTab — bot filter", () => { }); }); +// ── Update type filter options order ───────────────────────────────────────── + +describe("DependenciesTab — update type filter options order", () => { + it("shows options in risk order: Maintenance, Pin, Digest, Patch, Minor, Major, Other", () => { + vi.useFakeTimers(); + try { + const pr = makeMergeablePR(); + renderTab({ pullRequests: [pr] }); + const trigger = screen.getByRole("button", { name: /filter by update type/i }); + fireEvent.click(trigger); + vi.advanceTimersByTime(0); + const content = document.querySelector('[aria-label="Update type"]')!; + const optionLabels = Array.from(content.querySelectorAll("button")) + .map((b) => b.textContent?.replace(/^✓\s*/, "").trim()) + .filter((t): t is string => !!t && t !== "All"); + expect(optionLabels).toEqual(["Maintenance", "Pin", "Digest", "Patch", "Minor", "Major", "Other"]); + } finally { + vi.useRealTimers(); + } + }); +}); + +// ── Category classification ─────────────────────────────────────────────────── + +describe("DependenciesTab — category classification", () => { + it("digest category sorts between pin and patch, and renders a visible badge", () => { + const prPin = makeMergeablePR({ id: 7001, title: "chore(deps): pin dependencies" }); + const prDigest = makeMergeablePR({ id: 7002, title: "chore(deps): update rust crate pyo3 to v0.29.1" }); + const prPatch = makeMergeablePR({ id: 7003, title: "Bump axios from 0.27.1 to 0.27.2" }); + const depMeta = new Map([ + [prDigest.id, { updateType: "digest" as const, packageName: "pyo3", to: "v0.29.1" }], + ]); + + renderTab({ pullRequests: [prPatch, prPin, prDigest], depMeta }); + + expect(screen.getByText("digest")).toBeDefined(); + + const items = screen.getAllByRole("listitem"); + const categories = items + .map((item) => item.querySelector(".badge")?.textContent?.trim()) + .filter((c): c is string => !!c); + expect(categories).toEqual(["pin", "digest", "patch"]); + }); + + it("PR with unparseable title and no matching labels renders as 'other' (hidden badge), not 'maintenance'", () => { + const pr = makeMergeablePR({ title: "chore(deps): refresh vendored dependencies" }); + renderTab({ pullRequests: [pr] }); + expect(screen.queryByText("maintenance")).toBeNull(); + expect(screen.queryByText("other")).toBeNull(); + + const item = screen.getByRole("listitem"); + expect(item.querySelector(".badge")).toBeNull(); + }); + + it("PR with a lock-file-maintenance title still renders as 'maintenance' (regression guard)", () => { + const pr = makeMergeablePR({ title: "chore(deps): lock file maintenance" }); + renderTab({ pullRequests: [pr] }); + expect(screen.getByText("maintenance")).toBeDefined(); + }); + + it("PR with unparseable title and a 'digest' label renders category 'digest' with a visible badge", () => { + const pr = makeMergeablePR({ + title: "chore(deps): refresh vendored dependencies", + labels: [{ name: "digest", color: "1a7f37" }], + }); + renderTab({ pullRequests: [pr] }); + + const item = screen.getByRole("listitem"); + expect(item.querySelector(".badge")?.textContent?.trim()).toBe("digest"); + }); + + it("PR with unparseable title and a 'pin' label renders category 'pin' with a visible badge", () => { + const pr = makeMergeablePR({ + title: "chore(deps): refresh vendored dependencies", + labels: [{ name: "pin", color: "1a7f37" }], + }); + renderTab({ pullRequests: [pr] }); + + const item = screen.getByRole("listitem"); + expect(item.querySelector(".badge")?.textContent?.trim()).toBe("pin"); + }); + + it("PR with unparseable title and a 'maintenance' label renders category 'maintenance' with a visible badge", () => { + const pr = makeMergeablePR({ + title: "chore(deps): refresh vendored dependencies", + labels: [{ name: "maintenance", color: "1a7f37" }], + }); + renderTab({ pullRequests: [pr] }); + + const item = screen.getByRole("listitem"); + expect(item.querySelector(".badge")?.textContent?.trim()).toBe("maintenance"); + }); +}); + // ── Label filtering ────────────────────────────────────────────────────────── describe("DependenciesTab — label filtering", () => { diff --git a/tests/stores/view.test.ts b/tests/stores/view.test.ts index ba5710e0..d679d01b 100644 --- a/tests/stores/view.test.ts +++ b/tests/stores/view.test.ts @@ -948,6 +948,11 @@ describe("DependencyFiltersSchema", () => { const result = DependencyFiltersSchema.parse({ updateType: "major" }); expect(result.bot).toBe("all"); }); + + it("accepts 'digest' as a valid updateType value", () => { + const result = DependencyFiltersSchema.safeParse({ updateType: "digest", bot: "all" }); + expect(result.success).toBe(true); + }); }); describe("setTabFilter / resetAllTabFilters — dependencies", () => { From 7b533dbd12e16e4f99fde404b07bff02b486ecae Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Tue, 18 Aug 2026 09:12:13 -0400 Subject: [PATCH 2/3] fix(deps): recognize crate, docker, and generic title patterns Extends extractVersionInfo() with three new patterns for Renovate title phrasings not yet covered: Rust crate updates ("update [rust] crate X to vY"), Docker tag updates ("update X Docker tag to vY", verified against real production titles from backstage/mastodon/ Kyoo - depName comes before "Docker tag" per Renovate's commitMessageTopic template), and a generic single-target fallback ("update X to vY") for any remaining manager not explicitly covered. All three patterns guard the captured "to" value with a version-shape check, matching the existing depMatch/actionMatch patterns, so a non-version target (e.g. "latest") correctly falls through to null instead of producing a nonsensical result - letting the body-fetch fallback classify it instead. --- src/app/lib/dependency-detection.ts | 18 ++++++++ tests/lib/dependency-detection.test.ts | 60 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/app/lib/dependency-detection.ts b/src/app/lib/dependency-detection.ts index 99140459..2fd5f6a3 100644 --- a/src/app/lib/dependency-detection.ts +++ b/src/app/lib/dependency-detection.ts @@ -145,12 +145,30 @@ export function extractVersionInfo(title: string): VersionInfo | null { return { packageName: actionMatch[1]!, to: actionMatch[2]! }; } + // "Update (rust) crate X to vY" + const crateMatch = /^Update\s+(?:rust\s+)?crate\s+(.+?)\s+to\s+(v?[\w.\-+]+)/i.exec(body); + if (crateMatch && /^v?\d/.test(crateMatch[2]!)) { + return { packageName: crateMatch[1]!, to: crateMatch[2]! }; + } + + // "Update X Docker tag to vY" + const dockerMatch = /^Update\s+(.+?)\s+docker\s+tag\s+to\s+(v?[\w.\-+]+)/i.exec(body); + if (dockerMatch && /^v?\d/.test(dockerMatch[2]!)) { + return { packageName: dockerMatch[1]!, to: dockerMatch[2]! }; + } + // Generic "from A to B" anywhere const genericMatch = /\bfrom\s+([\w.\-+]+)\s+to\s+([\w.\-+]+)/i.exec(body); if (genericMatch) { return { from: genericMatch[1]!, to: genericMatch[2]!, updateType: semverUpdateType(genericMatch[1]!, genericMatch[2]!) ?? undefined }; } + // Generic "Update X to vY" (last resort, single-target version only) + const singleTargetMatch = /^Update\s+(.+?)\s+to\s+(v?[\w.\-+]+)$/i.exec(body); + if (singleTargetMatch && /^v?\d/.test(singleTargetMatch[2]!)) { + return { packageName: singleTargetMatch[1]!, to: singleTargetMatch[2]! }; + } + return null; } diff --git a/tests/lib/dependency-detection.test.ts b/tests/lib/dependency-detection.test.ts index f3bfd304..a0abf98b 100644 --- a/tests/lib/dependency-detection.test.ts +++ b/tests/lib/dependency-detection.test.ts @@ -209,6 +209,66 @@ describe("extractVersionInfo", () => { const result = extractVersionInfo("Bump lodash from 4.17.21 to 4.17.21"); expect(result).toEqual({ packageName: "lodash", from: "4.17.21", to: "4.17.21", updateType: undefined }); }); + + it("extracts package name and to-version for Renovate rust crate title", () => { + const result = extractVersionInfo("chore(deps): update rust crate pyo3 to v0.29.1"); + expect(result).toEqual({ packageName: "pyo3", to: "v0.29.1" }); + }); + + it("extracts package name and to-version for Renovate Docker tag title", () => { + const result = extractVersionInfo("chore(deps): update node Docker tag to v24.18.1"); + expect(result).toEqual({ packageName: "node", to: "v24.18.1" }); + }); + + it("returns exactly the depName (not 'depName Docker tag') for Docker tag title", () => { + const result = extractVersionInfo("chore(deps): update postgres Docker tag to v15"); + expect(result).toEqual({ packageName: "postgres", to: "v15" }); + }); + + it("does not match the crate pattern when the captured target is not version-shaped", () => { + const result = extractVersionInfo("chore(deps): update crate foo to latest"); + expect(result).toBeNull(); + }); + + it("does not match the Docker tag pattern when the captured target is not version-shaped", () => { + const result = extractVersionInfo("chore(deps): update node Docker tag to latest"); + expect(result).toBeNull(); + }); + + it("extracts package name and to-version via generic single-target fallback (renovate)", () => { + const result = extractVersionInfo("chore(deps): update renovate to v44"); + expect(result).toEqual({ packageName: "renovate", to: "v44" }); + }); + + it("extracts package name and to-version via generic single-target fallback (node)", () => { + const result = extractVersionInfo("chore(deps): update node to v24.18.1"); + expect(result).toEqual({ packageName: "node", to: "v24.18.1" }); + }); + + it("falls through to the generic single-target fallback, not the Docker/crate patterns, for titles without those keywords", () => { + const titles = ["chore(deps): update renovate to v44", "chore(deps): update node to v24.18.1"]; + for (const title of titles) { + expect(title.toLowerCase()).not.toContain("docker"); + expect(title.toLowerCase()).not.toContain("crate"); + } + expect(extractVersionInfo(titles[0]!)).toEqual({ packageName: "renovate", to: "v44" }); + expect(extractVersionInfo(titles[1]!)).toEqual({ packageName: "node", to: "v24.18.1" }); + }); + + it("does not match the generic single-target fallback when the captured target is not version-shaped", () => { + const result = extractVersionInfo("chore(deps): update the documentation to latest"); + expect(result).toBeNull(); + }); + + it("still prefers genericMatch's semver-diff path over the new single-target patterns when 'from' is present", () => { + // Synthetic "Update X from A to B"-style title (no real crate-manager sample of this shape was + // found) without the literal "crate"/"docker tag" keywords, since a title that combines those + // keywords with "from...to" hits the documented absorption caveat on the crate/docker patterns + // themselves (see Task 2 Step 1's ASSUMPTION note) rather than exercising this fallback-ordering + // regression check. + const result = extractVersionInfo("chore(deps): update pyo3 from 0.29.0 to 0.29.1"); + expect(result).toEqual({ from: "0.29.0", to: "0.29.1", updateType: "patch" }); + }); }); describe("stripVersionSpecifier", () => { From 8c90693ce6f94c290ddf4bf8789e5ee2d49c0c22 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Tue, 18 Aug 2026 09:13:57 -0400 Subject: [PATCH 3/3] fix(deps): bound GraphQL body-fetch requests with a timeout Fixes the root cause of dependency PRs permanently losing their type badges mid-session: fetchDepPRBodies/fetchDashboardIssueBodies had no timeout on their GraphQL calls. A hung request (suspected: octokit's throttling/retry queue stalling before fetch() dispatches) meant the awaited promise never settled, so DashboardPage's fetch-in-progress gate booleans - reset only in a finally block - never reset, blocking all future dependency-PR classification for the rest of the session. Adds a shared raceWithTimeout() helper: Promise.race against a 20s timeout + AbortController, with the timer cleared on whichever branch settles first. Promise.race is the load-bearing fix that guarantees the gate always resets; AbortController is best-effort cleanup layered on top, since it has no defined effect on a request still queued in the retry layer before fetch() is dispatched. An auth-clear race guard (getClient() === octokit) suppresses the post-timeout failure notification if the user has since logged out or re-authenticated, since the 20s window means the original session may no longer be active. Diagnostic logging is added to both functions and to DashboardPage's dep-PR-bodies effect to distinguish a genuine hang from a separate, still-open reactivity question about that effect's subscription tracking. --- .../components/dashboard/DashboardPage.tsx | 15 +++- src/app/services/api.ts | 69 ++++++++++++++-- tests/services/api-dashboard-bodies.test.ts | 63 ++++++++++++++- tests/services/api-dep-pr-bodies.test.ts | 80 ++++++++++++++++++- 4 files changed, 215 insertions(+), 12 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 99be3529..505fbb1e 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -1209,7 +1209,10 @@ export default function DashboardPage() { // classification survives page refresh without visual jank. createEffect(() => { if (!config.dependencies.enabled) return; - if (_fetchingDepBodies) return; + if (_fetchingDepBodies) { + console.debug("[dashboard] depBodies effect: skipped — fetch already in flight (this run's tracked deps are now narrowed to config.dependencies.enabled only)"); + return; + } const octokit = getClient(); if (!octokit) return; @@ -1217,13 +1220,19 @@ export default function DashboardPage() { const depPrs = dependencyPullRequests(); const visibleDepPrs = visibleDependencyPullRequests(); const toFetch = visibleDepPrs.filter((pr) => !meta.has(pr.id) && needsBodyFallback(pr)); - if (toFetch.length === 0) return; + if (toFetch.length === 0) { + console.debug("[dashboard] depBodies effect: nothing to fetch", { metaSize: meta.size, visibleDepPrCount: visibleDepPrs.length }); + return; + } _fetchingDepBodies = true; + const effectStart = Date.now(); + console.debug(`[dashboard] depBodies effect: fetch started for ${toFetch.length} PRs at ${effectStart}`); void (async () => { try { const nodeIds = toFetch.map((pr) => pr.nodeId!); const bodyMap = await fetchDepPRBodies(octokit, nodeIds); + console.debug(`[dashboard] depBodies effect: fetch resolved after ${Date.now() - effectStart}ms`, { requested: toFetch.length, returned: bodyMap.size }); if (bodyMap.size === 0) return; const merged = new Map(meta); @@ -1237,9 +1246,11 @@ export default function DashboardPage() { if (!depPrIds.has(k)) merged.delete(k); } setDepMeta(merged); + console.debug(`[dashboard] depBodies effect: depMeta updated, size=${merged.size}`); setTimeout(() => persistDepMeta(merged), 0); } finally { _fetchingDepBodies = false; + console.debug(`[dashboard] depBodies effect: guard released after ${Date.now() - effectStart}ms`); } })(); }); diff --git a/src/app/services/api.ts b/src/app/services/api.ts index bba40f31..bf5329b0 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1162,6 +1162,29 @@ export async function fetchPREnrichment( return { enrichments, errors }; } +// Shared timeout guard for body-fetch GraphQL calls. Prevents a hung request +// (e.g. octokit's secondary-rate-limit retry logic stalling indefinitely) +// from wedging the caller's fetch-in-progress gate. + +export const GRAPHQL_BODY_FETCH_TIMEOUT_MS = 20_000; + +export class GraphqlFetchTimeoutError extends Error {} + +export function raceWithTimeout(promise: Promise, ms: number, controller: AbortController): Promise { + let timeoutId: ReturnType | undefined; + + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + controller.abort(); + reject(new GraphqlFetchTimeoutError(`GraphQL request exceeded ${ms}ms`)); + }, ms); + }); + + return Promise.race([promise, timeoutPromise]).finally(() => { + clearTimeout(timeoutId); + }); +} + // ── Dashboard issue body fetch ──────────────────────────────────────────────── const DASHBOARD_ISSUE_BODIES_QUERY = ` @@ -1187,11 +1210,19 @@ export async function fetchDashboardIssueBodies( if (issueNodeIds.length === 0) return result; const batches = chunkArray(issueNodeIds, NODES_BATCH_SIZE); + let hadFailure = false; await Promise.allSettled(batches.map(async (batch) => { + const batchStart = Date.now(); + console.debug(`[api] dashboardBodies batch started (${batch.length} ids) at ${batchStart}`); + const controller = new AbortController(); try { - const response = await octokit.graphql( - DASHBOARD_ISSUE_BODIES_QUERY, - { ids: batch, request: { apiSource: "dashboardBodies" } } + const response = await raceWithTimeout( + octokit.graphql( + DASHBOARD_ISSUE_BODIES_QUERY, + { ids: batch, request: { apiSource: "dashboardBodies", signal: controller.signal } } + ), + GRAPHQL_BODY_FETCH_TIMEOUT_MS, + controller, ); if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit); for (const node of response.nodes) { @@ -1199,15 +1230,24 @@ export async function fetchDashboardIssueBodies( result.set(node.id, node.body); } } catch (err) { + hadFailure = true; + console.warn("[api] dashboardBodies batch failed or timed out:", err); + Sentry.captureException(err, { tags: { source: "dashboardBodies" } }); const partialErr = err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object" ? (err.data as Partial) : null; if (partialErr?.rateLimit) updateGraphqlRateLimit(partialErr.rateLimit); // Partial failures return null bodies — callers handle missing entries gracefully + } finally { + console.debug(`[api] dashboardBodies batch settled after ${Date.now() - batchStart}ms`); } })); + if (hadFailure && getClient() === octokit) { + pushNotification("dashboardBodies", "Some dependency dashboard data could not be loaded", "warning"); + } + return result; } @@ -1235,11 +1275,19 @@ export async function fetchDepPRBodies( if (prNodeIds.length === 0) return result; const batches = chunkArray(prNodeIds, NODES_BATCH_SIZE); + let hadFailure = false; await Promise.allSettled(batches.map(async (batch) => { + const batchStart = Date.now(); + console.debug(`[api] depPRBodies batch started (${batch.length} ids) at ${batchStart}`); + const controller = new AbortController(); try { - const response = await octokit.graphql( - DEP_PR_BODIES_QUERY, - { ids: batch, request: { apiSource: "depPRBodies" } } + const response = await raceWithTimeout( + octokit.graphql( + DEP_PR_BODIES_QUERY, + { ids: batch, request: { apiSource: "depPRBodies", signal: controller.signal } } + ), + GRAPHQL_BODY_FETCH_TIMEOUT_MS, + controller, ); if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit); for (const node of response.nodes) { @@ -1247,14 +1295,23 @@ export async function fetchDepPRBodies( result.set(node.databaseId, node.body); } } catch (err) { + hadFailure = true; + console.warn("[api] depPRBodies batch failed or timed out:", err); + Sentry.captureException(err, { tags: { source: "depPRBodies" } }); const partialErr = err && typeof err === "object" && "data" in err && err.data && typeof err.data === "object" ? (err.data as Partial) : null; if (partialErr?.rateLimit) updateGraphqlRateLimit(partialErr.rateLimit); + } finally { + console.debug(`[api] depPRBodies batch settled after ${Date.now() - batchStart}ms`); } })); + if (hadFailure && getClient() === octokit) { + pushNotification("depPRBodies", "Some dependency PR types could not be determined — badges may be missing", "warning"); + } + return result; } diff --git a/tests/services/api-dashboard-bodies.test.ts b/tests/services/api-dashboard-bodies.test.ts index e65a71d4..9a7f0fe3 100644 --- a/tests/services/api-dashboard-bodies.test.ts +++ b/tests/services/api-dashboard-bodies.test.ts @@ -1,13 +1,16 @@ import "fake-indexeddb/auto"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { fetchDashboardIssueBodies } from "../../src/app/services/api"; +import { fetchDashboardIssueBodies, GRAPHQL_BODY_FETCH_TIMEOUT_MS } from "../../src/app/services/api"; +import { getClient } from "../../src/app/services/github"; +import { pushNotification } from "../../src/app/lib/errors"; +import { captureException } from "@sentry/solid"; // ── Mocks ───────────────────────────────────────────────────────────────────── // updateGraphqlRateLimit lives in github.ts — mock the whole module const mockUpdateGraphqlRateLimit = vi.fn(); vi.mock("../../src/app/services/github", () => ({ - getClient: vi.fn(() => null), + getClient: vi.fn(), cachedRequest: vi.fn(), updateGraphqlRateLimit: (...args: unknown[]) => mockUpdateGraphqlRateLimit(...args), fetchRateLimitDetails: vi.fn(), @@ -28,6 +31,10 @@ vi.mock("../../src/app/lib/errors", () => ({ isMuted: vi.fn(() => false), })); +vi.mock("@sentry/solid", () => ({ + captureException: vi.fn(), +})); + // ── Helpers ─────────────────────────────────────────────────────────────────── // NODES_BATCH_SIZE is 100 (internal to api.ts — confirmed from fetchPREnrichment usage) @@ -248,3 +255,55 @@ describe("fetchDashboardIssueBodies — GraphQL error handling", () => { expect(result.get(`N_${NODES_BATCH_SIZE}`)).toBe(`body-N_${NODES_BATCH_SIZE}`); }); }); + +describe("fetchDashboardIssueBodies — hung request timeout", () => { + it("times out a never-resolving batch, warns, reports to Sentry, and notifies the user", async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const octokit = makeOctokit(() => new Promise(() => {})); + vi.mocked(getClient).mockReturnValue(octokit); + + const resultPromise = fetchDashboardIssueBodies(octokit, ["N_1"]); + await vi.advanceTimersByTimeAsync(GRAPHQL_BODY_FETCH_TIMEOUT_MS); + const result = await resultPromise; + + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + expect(warnSpy).toHaveBeenCalled(); + expect(captureException).toHaveBeenCalledWith( + expect.any(Error), + { tags: { source: "dashboardBodies" } } + ); + expect(pushNotification).toHaveBeenCalledWith( + "dashboardBodies", + expect.stringContaining("could not be loaded"), + "warning" + ); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it("suppresses the notification (but keeps diagnostics) when the client changed before the timeout fired", async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const octokit = makeOctokit(() => new Promise(() => {})); + vi.mocked(getClient).mockReturnValue(octokit); + + const resultPromise = fetchDashboardIssueBodies(octokit, ["N_1"]); + vi.mocked(getClient).mockReturnValue(null); + await vi.advanceTimersByTimeAsync(GRAPHQL_BODY_FETCH_TIMEOUT_MS); + await resultPromise; + + expect(warnSpy).toHaveBeenCalled(); + expect(captureException).toHaveBeenCalled(); + expect(pushNotification).not.toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); +}); diff --git a/tests/services/api-dep-pr-bodies.test.ts b/tests/services/api-dep-pr-bodies.test.ts index 93d69d1e..978bb8cd 100644 --- a/tests/services/api-dep-pr-bodies.test.ts +++ b/tests/services/api-dep-pr-bodies.test.ts @@ -1,10 +1,13 @@ import "fake-indexeddb/auto"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { fetchDepPRBodies } from "../../src/app/services/api"; +import { fetchDepPRBodies, raceWithTimeout, GRAPHQL_BODY_FETCH_TIMEOUT_MS } from "../../src/app/services/api"; +import { getClient } from "../../src/app/services/github"; +import { pushNotification } from "../../src/app/lib/errors"; +import { captureException } from "@sentry/solid"; const mockUpdateGraphqlRateLimit = vi.fn(); vi.mock("../../src/app/services/github", () => ({ - getClient: vi.fn(() => null), + getClient: vi.fn(), cachedRequest: vi.fn(), updateGraphqlRateLimit: (...args: unknown[]) => mockUpdateGraphqlRateLimit(...args), fetchRateLimitDetails: vi.fn(), @@ -25,6 +28,10 @@ vi.mock("../../src/app/lib/errors", () => ({ isMuted: vi.fn(() => false), })); +vi.mock("@sentry/solid", () => ({ + captureException: vi.fn(), +})); + const NODES_BATCH_SIZE = 100; function makeRateLimit() { @@ -193,3 +200,72 @@ describe("fetchDepPRBodies — error resilience", () => { expect(mockUpdateGraphqlRateLimit).toHaveBeenCalledWith(rl); }); }); + +describe("fetchDepPRBodies — hung request timeout", () => { + it("times out a never-resolving batch, warns, reports to Sentry, and notifies the user", async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const octokit = makeOctokit(() => new Promise(() => {})); + vi.mocked(getClient).mockReturnValue(octokit); + + const resultPromise = fetchDepPRBodies(octokit, ["N_1"]); + await vi.advanceTimersByTimeAsync(GRAPHQL_BODY_FETCH_TIMEOUT_MS); + const result = await resultPromise; + + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + expect(warnSpy).toHaveBeenCalled(); + expect(captureException).toHaveBeenCalledWith( + expect.any(Error), + { tags: { source: "depPRBodies" } } + ); + expect(pushNotification).toHaveBeenCalledWith( + "depPRBodies", + expect.stringContaining("could not be determined"), + "warning" + ); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it("suppresses the notification (but keeps diagnostics) when the client changed before the timeout fired", async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const octokit = makeOctokit(() => new Promise(() => {})); + vi.mocked(getClient).mockReturnValue(octokit); + + const resultPromise = fetchDepPRBodies(octokit, ["N_1"]); + vi.mocked(getClient).mockReturnValue(null); + await vi.advanceTimersByTimeAsync(GRAPHQL_BODY_FETCH_TIMEOUT_MS); + await resultPromise; + + expect(warnSpy).toHaveBeenCalled(); + expect(captureException).toHaveBeenCalled(); + expect(pushNotification).not.toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + vi.useRealTimers(); + } + }); +}); + +describe("raceWithTimeout", () => { + it("clears the timeout once the wrapped promise wins, so it never fires afterward", async () => { + vi.useFakeTimers(); + try { + const controller = new AbortController(); + const result = await raceWithTimeout(Promise.resolve("done"), GRAPHQL_BODY_FETCH_TIMEOUT_MS, controller); + expect(result).toBe("done"); + + await vi.advanceTimersByTimeAsync(GRAPHQL_BODY_FETCH_TIMEOUT_MS); + + expect(controller.signal.aborted).toBe(false); + } finally { + vi.useRealTimers(); + } + }); +});