diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a99914..bccf4a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,3 +14,5 @@ ## 0.1.0 - First release: tray app, native toasts, sounds, bell animation + +- 20 s timeout on GitHub requests; a hung request no longer stalls polling diff --git a/src/core/github.ts b/src/core/github.ts index 38ce490..fcd0c5e 100644 --- a/src/core/github.ts +++ b/src/core/github.ts @@ -6,6 +6,19 @@ import type { GhEvent } from "./types"; const API = "https://api.github.com"; +/** A request that never answers must not freeze the poll loop. */ +const TIMEOUT_MS = 20_000; + +async function timedFetch(input: string, init: RequestInit = {}): Promise { + const ctl = new AbortController(); + const timer = setTimeout(() => ctl.abort(), TIMEOUT_MS); + try { + return await fetch(input, { ...init, signal: ctl.signal }); + } finally { + clearTimeout(timer); + } +} + /** Turn a user-facing target string into the endpoint we hit. */ export function endpointFor(target: string, login: string | null): string | null { if (target === "@me") { @@ -54,7 +67,7 @@ export async function fetchEvents( token: string, etag: string | null, ): Promise { - const res = await fetch(url, { headers: headers(token, etag) }); + const res = await timedFetch(url, { headers: headers(token, etag) }); const num = (k: string) => { const v = res.headers.get(k); return v === null ? null : Number(v); @@ -84,7 +97,7 @@ export interface Profile { /** Validate a token by asking who it belongs to. */ export async function whoAmI(token: string): Promise { - const res = await fetch(`${API}/user`, { headers: headers(token) }); + const res = await timedFetch(`${API}/user`, { headers: headers(token) }); if (!res.ok) throw new GhError(res.status, await res.text().catch(() => ""), null, null); const data = (await res.json()) as { login: string; name: string | null; avatar_url: string | null }; return { login: data.login, name: data.name ?? null, avatarUrl: data.avatar_url ?? null }; @@ -115,7 +128,7 @@ export interface Suggestions { /** Smart defaults for onboarding: the user's busiest repos and their orgs. */ export async function fetchSuggestions(token: string): Promise { const get = async (path: string) => { - const res = await fetch(`${API}${path}`, { headers: headers(token) }); + const res = await timedFetch(`${API}${path}`, { headers: headers(token) }); if (!res.ok) throw new GhError(res.status, await res.text().catch(() => ""), null, null); return res.json(); }; diff --git a/src/core/i18n.ts b/src/core/i18n.ts index 9b51327..817ddbc 100644 --- a/src/core/i18n.ts +++ b/src/core/i18n.ts @@ -127,6 +127,7 @@ const dict = { // poller "poller.noToken": "Falta el token de GitHub", "poller.rateLimit": "Rate limit de GitHub", + "poller.noAccess": "sin acceso o no existe (¿el token tiene el permiso repo?)", "poller.more": "…y {n} evento más. Abre la app para verlos.", "poller.morePlural": "…y {n} eventos más. Abre la app para verlos.", // event phrases @@ -268,6 +269,7 @@ const dict = { "empty.body": "Pushes, PRs, reviews… from the repos you watch. The bell is ready.", "poller.noToken": "GitHub token missing", "poller.rateLimit": "GitHub rate limit", + "poller.noAccess": "no access or doesn't exist (does the token have the repo scope?)", "poller.more": "…and {n} more event. Open the app to see it.", "poller.morePlural": "…and {n} more events. Open the app to see them.", "ev.push.title": "{who} pushed to {repo}", diff --git a/src/core/poller.ts b/src/core/poller.ts index 5855780..35d9274 100644 --- a/src/core/poller.ts +++ b/src/core/poller.ts @@ -104,6 +104,7 @@ export class Poller { this.cb.onStatus({ kind: "polling" }); const fresh: Notice[] = []; let remaining: number | null = null; + const failed: string[] = []; for (const target of targets) { const url = endpointFor(target, login); @@ -126,9 +127,12 @@ export class Poller { }); return; } - const msg = e instanceof Error ? e.message : String(e); - this.cb.onStatus({ kind: "error", message: `${target}: ${msg}` }); - // keep going with the other targets, one bad repo shouldn't kill the loop + // keep going with the other targets, one bad repo shouldn't kill + // the loop, but don't hide it behind a green "ok" either + const msg = e instanceof GhError && e.status === 404 + ? t("poller.noAccess") + : e instanceof Error ? e.message : String(e); + failed.push(`${target}: ${msg}`); } } @@ -141,7 +145,11 @@ export class Poller { await this.fire(unique); this.cb.onNotices(unique); } - this.cb.onStatus({ kind: "ok", at: new Date(), remaining }); + if (failed.length) { + this.cb.onStatus({ kind: "error", message: failed.join(" · ") }); + } else { + this.cb.onStatus({ kind: "ok", at: new Date(), remaining }); + } } /** Returns notices for events newer than what we've seen for this target. */