diff --git a/README.md b/README.md index 30d988d..8619701 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,21 @@ completes in 5–17 s at 26–62 neurons per feedback, versus ~1 s and ~4.7 neur for the old 8B model. Queue consumers use `max_batch_size: 5`, so a batch stays well inside the invocation limit. +### フィードバック受付時の認証確認 + +`POST /postFeedback` では、キューへの投函前に `OCTOKIT_PAT` で +`TrainLCD/Issues` のリポジトリ情報を取得します。PAT 未設定・失効、GitHub の +HTTP エラー、5 秒のタイムアウト、Issues 無効時には、HTTP 503 と +`{ "error": { "status": "unavailable", "message": "Feedback service is temporarily unavailable. Please try again later." } }` +を返します。失敗した投稿はキューへ投函しません。 + +アプリの `useFeedback` は HTTP エラーを送信失敗として扱い、フィードバック画面で +エラーダイアログを表示します。成功応答はキューへの受付完了を示します。 +受付後の PAT 失効や Issue 作成時の検証エラーまで同期的に保証するものではありません。 +GitHub の Issue 作成失敗は、キューのログに HTTP ステータス、リクエストID、 +検証エラーの項目を残します。レート制限時は `Retry-After` と +`x-ratelimit-reset` に従って再試行を遅らせます。 + ### Dead letter queue `processFeedbackMessage()` rethrows on failure so the consumer can `retry()`, diff --git a/src/consumers/feedbackTriage.ts b/src/consumers/feedbackTriage.ts index de5858b..d8d64f2 100644 --- a/src/consumers/feedbackTriage.ts +++ b/src/consumers/feedbackTriage.ts @@ -3,6 +3,7 @@ * Discord 通知を行う。AI 呼び出し以外のロジックは旧 Cloud Functions から流用。 */ import dayjs from 'dayjs'; +import { githubIssueError } from '../lib/githubIssueError'; import type { AICategory, AIComponent, @@ -691,6 +692,7 @@ function githubPost(env: Env, path: string, body: unknown): Promise { method: 'post', headers: { Accept: 'application/vnd.github+json', + 'Content-Type': 'application/json', Authorization: `Bearer ${env.OCTOKIT_PAT ?? ''}`, 'X-GitHub-Api-Version': '2022-11-28', 'User-Agent': 'trainlcd-worker', @@ -1446,10 +1448,9 @@ export const processFeedbackMessage = async ( let publicIssueUrl = marker?.publicIssueUrl ?? null; if (!marker) { - try { - const res = await githubPost(env, `${INTERNAL_REPO}/issues`, { - title: aiReport.title ?? '要約未取得', - body: ` + const res = await githubPost(env, `${INTERNAL_REPO}/issues`, { + title: aiReport.title ?? '要約未取得', + body: ` ![Image](${imageUrl}) @@ -1495,49 +1496,41 @@ ${sentryEventId} ## レポーターUID ${reporterUid} `.trim(), - assignees: ['TinyKitten'], - milestone: null, - labels: [ - reportType === 'feedback' && - !aiReport.isSpam && - GITHUB_LABELS.FEEDBACK_TYPE, - reportType === 'crash' && GITHUB_LABELS.CRASH_TYPE, - appEdition === 'production' && GITHUB_LABELS.PRODUCTION_APP, - appEdition === 'canary' && GITHUB_LABELS.CANARY_APP, - appClip && GITHUB_LABELS.PLATFORM_APPCLIP, - aiReport.isSpam && GITHUB_LABELS.SPAM_TYPE, - (triageFailed || needsSpamReview) && GITHUB_LABELS.UNKNOWN_TYPE, - osNameLabel, - autoModeLabel, - categoryLabel, - triageLabel, - ].filter(Boolean), - }); - - if (res.status !== 201) { - console.error(await res.text().catch(() => '')); - throw new Error(`GitHub API failed with status ${res.status}`); - } + assignees: ['TinyKitten'], + milestone: null, + labels: [ + reportType === 'feedback' && + !aiReport.isSpam && + GITHUB_LABELS.FEEDBACK_TYPE, + reportType === 'crash' && GITHUB_LABELS.CRASH_TYPE, + appEdition === 'production' && GITHUB_LABELS.PRODUCTION_APP, + appEdition === 'canary' && GITHUB_LABELS.CANARY_APP, + appClip && GITHUB_LABELS.PLATFORM_APPCLIP, + aiReport.isSpam && GITHUB_LABELS.SPAM_TYPE, + (triageFailed || needsSpamReview) && GITHUB_LABELS.UNKNOWN_TYPE, + osNameLabel, + autoModeLabel, + categoryLabel, + triageLabel, + ].filter(Boolean), + }); - // ここから先は Issue 作成済み。throw して再試行させると重複起票になるため、 - // レスポンスの解析に失敗しても続行し、分かった範囲をマーカーに残す。 - const created = (await res.json().catch((err: unknown) => { - console.error('feedbackTriage: 起票レスポンスの解析に失敗', { - reportId: id, - error: err instanceof Error ? err.message : String(err), - }); - return null; - })) as { html_url?: string; number?: number } | null; - issueNumber = typeof created?.number === 'number' ? created.number : null; - issueUrl = - typeof created?.html_url === 'string' ? created.html_url : null; - } catch (err) { - // Issue 作成前の失敗。握りつぶすと queue ハンドラが ack してメッセージを失うため、 - // 再送出して再試行させる（この時点では Issue は作られていないので重複しない）。 - console.error(err); - throw err; + if (res.status !== 201) { + throw await githubIssueError(res); } + // ここから先は Issue 作成済み。throw して再試行させると重複起票になるため、 + // レスポンスの解析に失敗しても続行し、分かった範囲をマーカーに残す。 + const created = (await res.json().catch((err: unknown) => { + console.error('feedbackTriage: 起票レスポンスの解析に失敗', { + reportId: id, + error: err instanceof Error ? err.message : String(err), + }); + return null; + })) as { html_url?: string; number?: number } | null; + issueNumber = typeof created?.number === 'number' ? created.number : null; + issueUrl = typeof created?.html_url === 'string' ? created.html_url : null; + // 原因コンポーネントが特定できている場合のみ、該当の公開リポジトリにも起票する。 // 公開側に載せるのは管理 Issue 番号とチケットIDだけで、フィードバックの内容は含めない。 // 起票後は管理 Issue 側にもコメントでリンクを残し、双方向に追えるようにする。 diff --git a/src/index.test.ts b/src/index.test.ts new file mode 100644 index 0000000..9930619 --- /dev/null +++ b/src/index.test.ts @@ -0,0 +1,55 @@ +import { + FEEDBACK_RETRY_DELAY_SECONDS, + processFeedbackMessage, +} from './consumers/feedbackTriage'; +import worker from './index'; +import { GitHubIssueError } from './lib/githubIssueError'; +import type { Env, FeedbackQueueMessage } from './types'; + +jest.mock('./consumers/feedbackTriage', () => ({ + FEEDBACK_RETRY_DELAY_SECONDS: 90, + processFeedbackMessage: jest.fn(), +})); + +it('GitHub のエラー詳細をログに残し、解除時刻まで再試行を遅らせる', async () => { + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const ack = jest.fn(); + const retry = jest.fn(); + const batch = { + queue: 'feedback-triage', + messages: [ + { id: 'msg-1', body: { report: { id: 'report-1' } }, ack, retry }, + ], + } as unknown as MessageBatch; + try { + jest + .mocked(processFeedbackMessage) + .mockRejectedValueOnce( + new GitHubIssueError(403, 'request-1', 3600, { errors: [] }) + ); + await worker.queue?.(batch, {} as Env, {} as ExecutionContext); + expect(ack).not.toHaveBeenCalled(); + expect(retry).toHaveBeenCalledWith({ delaySeconds: 3600 }); + expect(spy).toHaveBeenCalledWith( + 'Queue message failed (feedback-triage):', + expect.objectContaining({ + status: 403, + errorMessage: 'GitHub API failed with status 403', + reportId: 'report-1', + requestId: 'request-1', + }) + ); + jest + .mocked(processFeedbackMessage) + .mockRejectedValueOnce(new Error('network error')); + await worker.queue?.(batch, {} as Env, {} as ExecutionContext); + expect(retry).toHaveBeenLastCalledWith({ + delaySeconds: FEEDBACK_RETRY_DELAY_SECONDS, + }); + jest.mocked(processFeedbackMessage).mockResolvedValueOnce(undefined); + await worker.queue?.(batch, {} as Env, {} as ExecutionContext); + expect(ack).toHaveBeenCalledTimes(1); + } finally { + spy.mockRestore(); + } +}); diff --git a/src/index.ts b/src/index.ts index e049e00..fc338cb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,6 +8,7 @@ import { processFeedbackMessage, } from './consumers/feedbackTriage'; import { withCallable } from './lib/callable'; +import { GitHubIssueError } from './lib/githubIssueError'; import { handleAuthToken } from './routes/auth'; import { handleMaintenanceConfig, handleRemoteConfig } from './routes/config'; import { handleFeedback } from './routes/feedback'; @@ -80,11 +81,24 @@ const worker: ExportedHandler = { await processFeedbackMessage(message.body, env); message.ack(); } catch (e) { - console.error(`Queue message failed (${batch.queue}):`, e); + const delaySeconds = Math.max( + FEEDBACK_RETRY_DELAY_SECONDS, + e instanceof GitHubIssueError ? (e.retryDelaySeconds ?? 0) : 0 + ); + console.error(`Queue message failed (${batch.queue}):`, { + messageId: message.id, + reportId: message.body?.report?.id, + errorName: e instanceof Error ? e.name : 'UnknownError', + errorMessage: e instanceof Error ? e.message : String(e), + ...(e instanceof GitHubIssueError + ? { status: e.status, requestId: e.requestId, details: e.details } + : {}), + delaySeconds, + }); // 遅延なしで再試行すると、processFeedbackMessage が起票直後に書いた // 冪等化マーカーを KV のネガティブキャッシュ越しに読めず、Issue を // 作り直してしまう。キャッシュが切れてから再試行させる。 - message.retry({ delaySeconds: FEEDBACK_RETRY_DELAY_SECONDS }); + message.retry({ delaySeconds }); } } }, diff --git a/src/lib/feedbackAvailability.ts b/src/lib/feedbackAvailability.ts new file mode 100644 index 0000000..cdd022a --- /dev/null +++ b/src/lib/feedbackAvailability.ts @@ -0,0 +1,55 @@ +import type { Env } from '../types'; +import { CallableError } from './callable'; + +/** キュー受付前に、起票先へ認証できることを確認する。成功はキャッシュしない。 */ +export async function ensureFeedbackAvailable(env: Env): Promise { + const unavailable = () => + new CallableError( + 'unavailable', + 'Feedback service is temporarily unavailable. Please try again later.' + ); + if (!env.OCTOKIT_PAT?.trim()) { + console.error('Feedback intake unavailable', { + reason: 'OCTOKIT_PAT is not set', + }); + throw unavailable(); + } + let response: Response; + try { + // Issue 本文は取得せず、リポジトリへの認証・アクセスを検証する。 + response = await fetch('https://api.github.com/repos/TrainLCD/Issues', { + headers: { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${env.OCTOKIT_PAT}`, + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'trainlcd-worker', + }, + signal: AbortSignal.timeout(5000), + }); + } catch { + console.error('Feedback intake unavailable', { + reason: 'GitHub request failed or timed out', + }); + throw unavailable(); + } + if (!response.ok) { + console.error('Feedback intake unavailable', { + status: response.status, + requestId: response.headers.get('x-github-request-id'), + }); + await response.body?.cancel().catch(() => {}); + throw unavailable(); + } + const raw: unknown = await response.json().catch(() => null); + if ( + !raw || + typeof raw !== 'object' || + !('has_issues' in raw) || + raw.has_issues !== true + ) { + console.error('Feedback intake unavailable', { + reason: 'GitHub issue repository is unavailable', + }); + throw unavailable(); + } +} diff --git a/src/lib/githubIssueError.test.ts b/src/lib/githubIssueError.test.ts new file mode 100644 index 0000000..4518ec3 --- /dev/null +++ b/src/lib/githubIssueError.test.ts @@ -0,0 +1,122 @@ +import { githubIssueError } from './githubIssueError'; + +const response = (status: number, headers = {}, body: unknown = {}) => + new Response(JSON.stringify(body), { status, headers }); + +describe('githubIssueError', () => { + it('ステータスとリクエストIDを残し、検証エラーの投稿内容を除外する', async () => { + const error = await githubIssueError( + response( + 422, + { 'x-github-request-id': 'request-1' }, + { + message: 'private input', + errors: [ + { + resource: 'Issue', + field: 'body', + code: 'custom', + value: 'private', + message: 'private', + }, + ], + } + ) + ); + expect(error.message).toBe('GitHub API failed with status 422'); + expect(error.requestId).toBe('request-1'); + expect(error.details).toEqual({ + errors: [{ resource: 'Issue', field: 'body', code: 'custom' }], + }); + expect(JSON.stringify(error)).not.toContain('private'); + expect(error.retryDelaySeconds).toBeNull(); + }); + + it('レート制限解除時刻まで待つ', async () => { + const error = await githubIssueError( + response(403, { + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': '4600', + 'retry-after': '120', + }), + 1000_000 + ); + expect(error.retryDelaySeconds).toBe(3600); + }); + + it('secondary rate limit と Retry-After を扱う', async () => { + expect( + ( + await githubIssueError( + response(403, {}, { message: 'secondary rate limit' }) + ) + ).retryDelaySeconds + ).toBe(60); + expect( + (await githubIssueError(response(429, { 'retry-after': '180' }))) + .retryDelaySeconds + ).toBe(180); + }); + + it.each([403, 429])( + 'HTTP %s の HTTP-date を再試行時刻として扱う', + async (status) => { + const now = Date.UTC(2026, 9, 14, 0, 0, 0, 500); + const retryAt = new Date(now + 180_000).toUTCString(); + const error = await githubIssueError( + response(status, { 'retry-after': retryAt }), + now + ); + // HTTP-date の秒精度で 179.5 秒先を示す。端数は切り上げる。 + expect(error.retryDelaySeconds).toBe(180); + } + ); + + it('HTTP-date とレート制限解除時刻の遅い方まで待つ', async () => { + const now = Date.UTC(2026, 9, 14); + const error = await githubIssueError( + response(403, { + 'retry-after': new Date(now + 180_000).toUTCString(), + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': String(now / 1000 + 3600), + }), + now + ); + expect(error.retryDelaySeconds).toBe(3600); + }); + + it.each(['invalid-date', '', 'Wed, 14 Oct 2026 00:00:00 GMT'])( + '無効・空・過去の Retry-After (%s) は既定の待機時間を使う', + async (retryAfter) => { + const now = Date.UTC(2026, 9, 14, 1); + const error = await githubIssueError( + response(429, { 'retry-after': retryAfter }), + now + ); + expect(error.retryDelaySeconds).toBe(60); + } + ); + + it('無効な Retry-After だけで権限エラーをレート制限にしない', async () => { + const error = await githubIssueError( + response(403, { 'retry-after': 'invalid-date' }) + ); + expect(error.retryDelaySeconds).toBeNull(); + }); + + it('認証・権限エラーをレート制限として扱わない', async () => { + for (const status of [401, 403, 404, 500]) { + expect( + (await githubIssueError(response(status))).retryDelaySeconds + ).toBeNull(); + } + }); + + it('JSONでない障害応答でもステータスを失わない', async () => { + const error = await githubIssueError( + new Response('Bad Gateway', { status: 502 }) + ); + expect(error.status).toBe(502); + expect(error.details).toEqual({ errors: [] }); + }); +}); diff --git a/src/lib/githubIssueError.ts b/src/lib/githubIssueError.ts new file mode 100644 index 0000000..79c3862 --- /dev/null +++ b/src/lib/githubIssueError.ts @@ -0,0 +1,72 @@ +/** Issue 作成失敗の診断情報。投稿本文や認証情報は保持しない。 */ +export class GitHubIssueError extends Error { + constructor( + readonly status: number, + readonly requestId: string | null, + readonly retryDelaySeconds: number | null, + readonly details: unknown + ) { + super(`GitHub API failed with status ${status}`); + this.name = 'GitHubIssueError'; + } +} + +/** + * Issue 作成失敗の応答から、投稿内容を含まない診断情報と再試行遅延を取得する。 + * Retry-After は秒数・HTTP-date の両形式を扱い、遅延をキューの上限に収める。 + */ +export async function githubIssueError( + response: Response, + now = Date.now() +): Promise { + // errors[].value / message は入力を含み得るため記録しない。 + const raw: unknown = await response.json().catch(() => null); + const body = raw && typeof raw === 'object' ? raw : {}; + const errors = + 'errors' in body && Array.isArray(body.errors) + ? body.errors.map((error: unknown) => { + if (!error || typeof error !== 'object') return {}; + return Object.fromEntries( + ['resource', 'field', 'code'].flatMap((key) => { + const value = (error as Record)[key]; + return typeof value === 'string' + ? [[key, value.slice(0, 100)]] + : []; + }) + ); + }) + : []; + const retryAfterRaw = response.headers.get('retry-after'); + let retryAfter = Number(retryAfterRaw); + if (retryAfterRaw && !Number.isFinite(retryAfter)) { + const retryAt = Date.parse(retryAfterRaw); + retryAfter = Number.isFinite(retryAt) + ? Math.max(0, (retryAt - now) / 1000) + : Number.NaN; + } + const reset = Number(response.headers.get('x-ratelimit-reset')); + const rateLimited = + response.status === 429 || + (response.status === 403 && + (response.headers.get('x-ratelimit-remaining') === '0' || + retryAfter > 0 || + ('message' in body && + typeof body.message === 'string' && + /rate limit/i.test(body.message)))); + const delay = rateLimited + ? Math.max( + 60, + Number.isFinite(retryAfter) ? retryAfter : 0, + response.headers.get('x-ratelimit-remaining') === '0' && + Number.isFinite(reset) + ? reset - now / 1000 + : 0 + ) + : null; + return new GitHubIssueError( + response.status, + response.headers.get('x-github-request-id'), + delay === null ? null : Math.min(43200, Math.ceil(delay)), + { errors } + ); +} diff --git a/src/routes/feedback.test.ts b/src/routes/feedback.test.ts index 6dbf8e6..b1b0221 100644 --- a/src/routes/feedback.test.ts +++ b/src/routes/feedback.test.ts @@ -1,4 +1,4 @@ -import { CallableError } from '../lib/callable'; +import { CallableError, withCallable } from '../lib/callable'; import type { Env } from '../types'; import { handleFeedback } from './feedback'; @@ -7,7 +7,10 @@ jest.mock('../lib/auth/session', () => ({ })); const send = jest.fn(async (_message: unknown) => undefined); -const env = { FEEDBACK_QUEUE: { send } } as unknown as Env; +const env = { + OCTOKIT_PAT: 'test-pat', + FEEDBACK_QUEUE: { send }, +} as unknown as Env; const buildRequest = (report: unknown): Request => new Request('https://example.com/postFeedback', { @@ -27,10 +30,81 @@ const baseReport = { }; describe('handleFeedback', () => { + const originalFetch = global.fetch; + beforeEach(() => { + global.fetch = jest.fn( + async () => + new Response( + JSON.stringify({ has_issues: true, permissions: { push: true } }) + ) + ) as typeof fetch; + }); + afterEach(() => { + global.fetch = originalFetch; jest.clearAllMocks(); }); + it.each([401, 403, 404, 429, 500])( + 'returns an app-visible 503 for GitHub HTTP %s without queueing', + async (status) => { + global.fetch = jest.fn( + async () => new Response(null, { status }) + ) as typeof fetch; + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { + const res = await withCallable(() => + handleFeedback(buildRequest(baseReport), env) + ); + expect(res.status).toBe(503); + expect(await res.json()).toMatchObject({ + error: { status: 'unavailable' }, + }); + expect(send).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + } + ); + + it('returns 503 on a missing PAT or network failure without queueing', async () => { + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { + const missing = await withCallable(() => + handleFeedback(buildRequest(baseReport), { ...env, OCTOKIT_PAT: '' }) + ); + expect(missing.status).toBe(503); + expect(global.fetch).not.toHaveBeenCalled(); + global.fetch = jest.fn().mockRejectedValue(new Error('network failure')); + const failed = await withCallable(() => + handleFeedback(buildRequest(baseReport), env) + ); + expect(failed.status).toBe(503); + expect(send).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + }); + + it.each([null, { has_issues: false }])( + 'rejects an unavailable issue repository', + async (body) => { + global.fetch = jest.fn( + async () => new Response(JSON.stringify(body)) + ) as typeof fetch; + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { + const res = await withCallable(() => + handleFeedback(buildRequest(baseReport), env) + ); + expect(res.status).toBe(503); + expect(send).not.toHaveBeenCalled(); + } finally { + spy.mockRestore(); + } + } + ); + // 文字数の下限は撤廃したため、短い本文もそのままキューへ流す it('queues a short report', async () => { const res = await handleFeedback(buildRequest(baseReport), env); diff --git a/src/routes/feedback.ts b/src/routes/feedback.ts index 5a9cccc..0f43d8a 100644 --- a/src/routes/feedback.ts +++ b/src/routes/feedback.ts @@ -5,6 +5,7 @@ import { callableSuccess, parseCallableData, } from '../lib/callable'; +import { ensureFeedbackAvailable } from '../lib/feedbackAvailability'; import type { Report } from '../models/feedback'; import type { Env } from '../types'; @@ -25,6 +26,10 @@ export const handleFeedback = async ( throw new CallableError('invalid-argument', 'report.description required'); } + // 認証障害をキュー内だけで検知するとアプリに成功を返してしまうため、 + // PAT 失効・起票先へのアクセス障害は投函前に 503 として返す。 + await ensureFeedbackAvailable(env); + // reporterUid はクライアント申告を信用せず、検証済みトークンの sub で上書きする。 // （他ユーザーの UID を名乗って Issue/Discord に載せるなりすましを防ぐ） const verifiedReport: Report = { ...report, reporterUid: uid };