feat(repo): add Octokit request timeouts and retry UI
Before, every GitHub call awaited indefinitely with no AbortSignal, so a "lie-fi" connection (technically online, effectively dead) left FluxNote stuck on the skeleton loader and the freshness badge spinning forever. Inject an 8s AbortSignal.timeout via octokit.hook.before (20s override on the recursive tree fetch), classify failures in the userRepo store as auth vs network, and surface a "Couldn't reach GitHub" retry button when no cached content is available.
This commit is contained in:
@@ -73,6 +73,10 @@ watch(
|
||||
{ immediate: true }
|
||||
)
|
||||
|
||||
const retryLoad = () => {
|
||||
store.setUserRepo(props.user, props.repo)
|
||||
}
|
||||
|
||||
onMounted(() => visitRepo())
|
||||
|
||||
onUnmounted(() => {
|
||||
@@ -102,6 +106,13 @@ onUnmounted(() => {
|
||||
</div>
|
||||
<slot />
|
||||
<skeleton-loader v-if="isLoading" />
|
||||
<div
|
||||
v-else-if="withContent && !hasContent && store.loadError === 'network'"
|
||||
class="repo-network-error"
|
||||
>
|
||||
<p>Couldn't reach GitHub. Check your connection.</p>
|
||||
<button class="btn btn-primary" @click="retryLoad">Retry</button>
|
||||
</div>
|
||||
<div v-else-if="withContent && !hasContent" class="repo-not-found">
|
||||
<template v-if="isLogged">
|
||||
<p>This repository is not accessible.</p>
|
||||
@@ -198,7 +209,8 @@ $header-height: 40px;
|
||||
}
|
||||
}
|
||||
|
||||
.repo-not-found {
|
||||
.repo-not-found,
|
||||
.repo-network-error {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
align-items: center;
|
||||
|
||||
62
src/modules/repo/services/octo.spec.ts
Normal file
62
src/modules/repo/services/octo.spec.ts
Normal file
@@ -0,0 +1,62 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
|
||||
|
||||
vi.mock("@/modules/user/service/signIn", () => ({
|
||||
getAccessToken: vi.fn().mockResolvedValue({ token: "test-token" })
|
||||
}))
|
||||
|
||||
import { DEFAULT_OCTOKIT_TIMEOUT_MS, getOctokit } from "./octo"
|
||||
|
||||
const okResponse = () =>
|
||||
new Response("{}", {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" }
|
||||
})
|
||||
|
||||
describe("getOctokit timeout hook", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it("injects an AbortSignal.timeout into requests by default", async () => {
|
||||
const timeoutSpy = vi.spyOn(AbortSignal, "timeout")
|
||||
let receivedSignal: AbortSignal | undefined
|
||||
const mockFetch = vi.fn(async (_url: string, init: RequestInit) => {
|
||||
receivedSignal = init.signal ?? undefined
|
||||
return okResponse()
|
||||
})
|
||||
|
||||
const octokit = await getOctokit()
|
||||
await octokit.request("GET /user", {
|
||||
request: { fetch: mockFetch as unknown as typeof fetch }
|
||||
})
|
||||
|
||||
expect(timeoutSpy).toHaveBeenCalledWith(DEFAULT_OCTOKIT_TIMEOUT_MS)
|
||||
expect(receivedSignal).toBeInstanceOf(AbortSignal)
|
||||
})
|
||||
|
||||
it("respects a caller-provided signal and does not overwrite it", async () => {
|
||||
const timeoutSpy = vi.spyOn(AbortSignal, "timeout")
|
||||
const controller = new AbortController()
|
||||
let receivedSignal: AbortSignal | undefined
|
||||
const mockFetch = vi.fn(async (_url: string, init: RequestInit) => {
|
||||
receivedSignal = init.signal ?? undefined
|
||||
return okResponse()
|
||||
})
|
||||
|
||||
const octokit = await getOctokit()
|
||||
await octokit.request("GET /user", {
|
||||
request: {
|
||||
fetch: mockFetch as unknown as typeof fetch,
|
||||
signal: controller.signal
|
||||
}
|
||||
})
|
||||
|
||||
expect(timeoutSpy).not.toHaveBeenCalled()
|
||||
expect(receivedSignal).toBe(controller.signal)
|
||||
})
|
||||
|
||||
})
|
||||
@@ -2,10 +2,21 @@ import { Octokit } from "@octokit/rest"
|
||||
|
||||
import { getAccessToken } from "@/modules/user/service/signIn"
|
||||
|
||||
export const DEFAULT_OCTOKIT_TIMEOUT_MS = 8_000
|
||||
|
||||
export const getOctokit = async (): Promise<Octokit> => {
|
||||
const response = await getAccessToken()
|
||||
|
||||
return new Octokit({
|
||||
const octokit = new Octokit({
|
||||
auth: response?.token ?? ""
|
||||
})
|
||||
|
||||
octokit.hook.before("request", (options) => {
|
||||
options.request ??= {}
|
||||
if (!options.request.signal) {
|
||||
options.request.signal = AbortSignal.timeout(DEFAULT_OCTOKIT_TIMEOUT_MS)
|
||||
}
|
||||
})
|
||||
|
||||
return octokit
|
||||
}
|
||||
|
||||
@@ -30,7 +30,8 @@ export const getFiles = async (
|
||||
owner,
|
||||
repo,
|
||||
tree_sha: lastCommit.commit.tree.sha,
|
||||
recursive: "true"
|
||||
recursive: "true",
|
||||
request: { signal: AbortSignal.timeout(20_000) }
|
||||
}
|
||||
)
|
||||
|
||||
@@ -82,6 +83,10 @@ export const getMainReadme = async (owner: string, repo: string) => {
|
||||
if (cachedReadme) {
|
||||
return render(cachedReadme.content)
|
||||
}
|
||||
|
||||
// No cached fallback — surface the error so the caller can classify
|
||||
// network/timeout failures vs auth/404 and tailor the UI accordingly.
|
||||
throw error
|
||||
}
|
||||
|
||||
return null
|
||||
|
||||
@@ -14,6 +14,8 @@ import {
|
||||
} from "@/modules/repo/services/repo"
|
||||
import { refreshToken } from "@/modules/user/service/signIn"
|
||||
|
||||
export type RepoLoadError = "auth" | "network" | null
|
||||
|
||||
interface State {
|
||||
user: string
|
||||
repo: string
|
||||
@@ -21,9 +23,20 @@ interface State {
|
||||
readme?: string | null
|
||||
userSettings?: UserSettings | null
|
||||
needToLogin: boolean
|
||||
loadError: RepoLoadError
|
||||
_requestId: number
|
||||
}
|
||||
|
||||
const classifyLoadError = (error: unknown): "auth" | "network" => {
|
||||
if (error && typeof error === "object") {
|
||||
const e = error as { name?: string; status?: number }
|
||||
if (e.name === "TimeoutError" || e.name === "AbortError") return "network"
|
||||
if (e.status === 401 || e.status === 403 || e.status === 404) return "auth"
|
||||
if (typeof e.status === "number" && e.status >= 500) return "network"
|
||||
}
|
||||
return "network"
|
||||
}
|
||||
|
||||
export const useUserRepoStore = defineStore("USER_REPO_STATE", {
|
||||
state: (): State => ({
|
||||
user: "",
|
||||
@@ -32,6 +45,7 @@ export const useUserRepoStore = defineStore("USER_REPO_STATE", {
|
||||
readme: undefined,
|
||||
userSettings: undefined,
|
||||
needToLogin: false,
|
||||
loadError: null,
|
||||
_requestId: 0
|
||||
}),
|
||||
actions: {
|
||||
@@ -63,6 +77,7 @@ export const useUserRepoStore = defineStore("USER_REPO_STATE", {
|
||||
const requestId = ++this._requestId
|
||||
this.user = user
|
||||
this.repo = repo
|
||||
this.loadError = null
|
||||
|
||||
let lsLayout: Partial<UserSettings> = {}
|
||||
try {
|
||||
@@ -163,14 +178,29 @@ export const useUserRepoStore = defineStore("USER_REPO_STATE", {
|
||||
_id: userSettingsId
|
||||
})
|
||||
})
|
||||
.catch((error) => {
|
||||
if (requestId !== this._requestId) return
|
||||
console.warn("getFiles failed", error)
|
||||
this.loadError = classifyLoadError(error)
|
||||
})
|
||||
|
||||
getCachedMainReadme(user, repo).then(async (cachedReadme) => {
|
||||
getCachedMainReadme(user, repo)
|
||||
.then(async (cachedReadme) => {
|
||||
if (requestId !== this._requestId) return
|
||||
if (cachedReadme) this.readme = cachedReadme
|
||||
const fetched = await getMainReadme(user, repo)
|
||||
if (requestId !== this._requestId) return
|
||||
this.readme = fetched
|
||||
})
|
||||
.catch((error) => {
|
||||
if (requestId !== this._requestId) return
|
||||
console.warn("getMainReadme failed", error)
|
||||
// Only surface the error UI if we have nothing cached to display.
|
||||
if (!this.readme) {
|
||||
this.readme = null
|
||||
this.loadError = classifyLoadError(error)
|
||||
}
|
||||
})
|
||||
},
|
||||
addFile(file: RepoFile) {
|
||||
if (!file.sha) {
|
||||
|
||||
Reference in New Issue
Block a user