-
Notifications
You must be signed in to change notification settings - Fork 221
♻️ Extract GitHub API utilities and integrate token refresh #3773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 20 commits
e3dcb15
f433134
162a684
437fb3c
8f4ded6
b3aedd9
183f7d5
9b7b704
b9d4801
896dbbf
13f2557
f9d22ce
0cf8ebe
5b08f37
cc1a9a2
e4bc3e1
498d126
370ba03
07e4cd0
7fb689b
0aeeaa0
61e3146
aece9bd
164db73
d80f93e
e804aab
69c832f
85635de
dce67e7
eb68438
8541e7f
4af40bd
d3e80e7
972b2c9
8793232
663d433
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { fromAsyncThrowable } from '@liam-hq/neverthrow' | ||
| import { NextResponse } from 'next/server' | ||
| import { clearTokens } from '../../../../libs/github/cookie' | ||
|
|
||
| export async function POST() { | ||
| await fromAsyncThrowable(clearTokens)().match( | ||
| () => undefined, | ||
| () => undefined, | ||
| ) | ||
| return NextResponse.json({ ok: true }) | ||
| } | ||
|
MH4GF marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,143 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fromPromise, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fromValibotSafeParse, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type Result, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '@liam-hq/neverthrow' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as Sentry from '@sentry/nextjs' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { ResultAsync as NeverthrowResultAsync } from 'neverthrow' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { err, ok, ResultAsync } from 'neverthrow' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { NextResponse } from 'next/server' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as v from 'valibot' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { TokenPayload } from '../../../../../libs/github/cookie' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readRefreshToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| writeAccessToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| writeRefreshToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '../../../../../libs/github/cookie' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const getClientCredentials = (): Result< | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { clientId: string; clientSecret: string }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Error | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const clientId = process.env.GITHUB_CLIENT_ID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const clientSecret = process.env.GITHUB_CLIENT_SECRET | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!clientId || !clientSecret) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return err(new Error('Missing GitHub OAuth client credentials')) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ok({ clientId, clientSecret }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type RefreshAccessTokenResult = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| accessToken: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshToken: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshTokenExpiresIn: number | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| newExpiresAt: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // GitHub OAuth token refresh response schema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const GitHubTokenRefreshResponseSchema = v.object({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| access_token: v.string(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| token_type: v.string(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope: v.optional(v.string()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expires_in: v.number(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refresh_token: v.string(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refresh_token_expires_in: v.optional(v.number()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function refreshAccessToken( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshToken: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientSecret: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): ResultAsync<RefreshAccessTokenResult, Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = new URLSearchParams({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client_id: clientId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client_secret: clientSecret, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| grant_type: 'refresh_token', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refresh_token: refreshToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }).toString() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResultAsync.fromPromise( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fetch('https://github.com/login/oauth/access_token', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| method: 'POST', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Accept: 'application/json', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (e) => (e instanceof Error ? e : new Error(String(e))), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .andThen((res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!res.ok) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return err(new Error(`GitHub token refresh failed: ${res.status}`)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ResultAsync.fromPromise(res.json() as Promise<unknown>, (e) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| e instanceof Error ? e : new Error(String(e)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .andThen((data) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fromValibotSafeParse(GitHubTokenRefreshResponseSchema, data), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((data) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const now = Date.now() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const newExpiresAt = new Date( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| now + Math.max(0, data.expires_in) * 1000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).toISOString() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const refreshTokenExpiresIn = data.refresh_token_expires_in ?? 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| accessToken: data.access_token, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshToken: data.refresh_token, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshTokenExpiresIn, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| newExpiresAt, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainCritical: Defaulting refresh_token_expires_in to 0 causes immediate cookie expiry. Line 86 sets GitHub's documentation indicates .map((data) => {
const now = Date.now()
const newExpiresAt = new Date(
now + Math.max(0, data.expires_in) * 1000,
).toISOString()
- const refreshTokenExpiresIn = data.refresh_token_expires_in ?? 0
+
+ // GitHub should provide refresh_token_expires_in; if missing, treat as error
+ // or use a safe fallback (e.g., 6 months = 15552000 seconds)
+ if (!data.refresh_token_expires_in) {
+ throw new Error('GitHub did not provide refresh_token_expires_in')
+ }
+ const refreshTokenExpiresIn = data.refresh_token_expires_in
return {
accessToken: data.access_token,
refreshToken: data.refresh_token,
refreshTokenExpiresIn,
newExpiresAt,
}
})Alternatively, use a safe default like 6 months (15552000 seconds) if you want to handle GitHub's omission gracefully. Run this script to check GitHub's documentation on the expected behavior: 🌐 Web query: 💡 Result: Short answer: No — GitHub only returns refresh_token_expires_in when your app is using expiring user access tokens. When returned its value is 15897600 seconds (GitHub describes this as 6 months). [1][2] Details:
Sources: GitHub docs — "Generating a user access token for a GitHub App" and "Refreshing user access tokens". [1][2] Now I need to examine the actual route.ts file to understand how Length of output: 168 Length of output: 4240 Critical: Defaulting refresh_token_expires_in to 0 causes the refresh token cookie to expire immediately. Line 86 sets GitHub enables token expiration by default and when enabled, refresh tokens expire after six months (15897600 seconds). The schema marks Consider this fix: .map((data) => {
const now = Date.now()
const newExpiresAt = new Date(
now + Math.max(0, data.expires_in) * 1000,
).toISOString()
- const refreshTokenExpiresIn = data.refresh_token_expires_in ?? 0
+ // If refresh_token_expires_in is missing, use GitHub's default of 6 months (15897600 seconds)
+ const refreshTokenExpiresIn = data.refresh_token_expires_in ?? 15897600
return {
accessToken: data.access_token,
refreshToken: data.refresh_token,
refreshTokenExpiresIn,
newExpiresAt,
}
})📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function handler(): Promise<NextResponse> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 1) Load client credentials | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const creds = getClientCredentials() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (creds.isErr()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Sentry.captureException(creds.error, { tags: { area: 'refresh_api' } }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return NextResponse.json( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { ok: false, error: 'server_misconfigured' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { status: 500 }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { clientId, clientSecret } = creds.value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 2) Read refresh token and refresh via GitHub | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const refreshTokenResult: NeverthrowResultAsync<TokenPayload, Error> = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readRefreshToken() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const result = await refreshTokenResult | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .andThen(({ token }: TokenPayload) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshAccessToken(token, clientId, clientSecret), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .andThen((refreshed: RefreshAccessTokenResult) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rtei = refreshed.refreshTokenExpiresIn | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const refreshTtlMs = rtei * 1000 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const refreshExpiresAt = new Date(Date.now() + refreshTtlMs).toISOString() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Ensure cookie writes complete and propagate failures | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fromPromise( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Promise.all([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| writeAccessToken(refreshed.accessToken, refreshed.newExpiresAt), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| writeRefreshToken(refreshed.refreshToken, refreshExpiresAt), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).map(() => NextResponse.json({ ok: true })) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (result.isErr()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Sentry.captureException(result.error, { tags: { area: 'refresh_api' } }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return NextResponse.json( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { ok: false, error: 'refresh_failed' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { status: 401 }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result.value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function POST() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return handler() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,13 @@ | ||||||||||||||||||
| import { fromAsyncThrowable } from '@liam-hq/neverthrow' | ||||||||||||||||||
| import * as Sentry from '@sentry/nextjs' | ||||||||||||||||||
| import { NextResponse } from 'next/server' | ||||||||||||||||||
| import { ensureUserHasOrganization } from '../../../../components/LoginPage/services/ensureUserHasOrganization' | ||||||||||||||||||
| import { sanitizeReturnPath } from '../../../../components/LoginPage/services/validateReturnPath' | ||||||||||||||||||
| import { createClient } from '../../../../libs/db/server' | ||||||||||||||||||
| import { | ||||||||||||||||||
| writeAccessToken, | ||||||||||||||||||
| writeRefreshToken, | ||||||||||||||||||
| } from '../../../../libs/github/cookie' | ||||||||||||||||||
|
|
||||||||||||||||||
| export async function GET(request: Request) { | ||||||||||||||||||
| const { searchParams, origin } = new URL(request.url) | ||||||||||||||||||
|
|
@@ -11,9 +17,44 @@ export async function GET(request: Request) { | |||||||||||||||||
|
|
||||||||||||||||||
| if (code) { | ||||||||||||||||||
| const supabase = await createClient() | ||||||||||||||||||
| const { error } = await supabase.auth.exchangeCodeForSession(code) | ||||||||||||||||||
| const { data, error } = await supabase.auth.exchangeCodeForSession(code) | ||||||||||||||||||
| if (!error) { | ||||||||||||||||||
| await ensureUserHasOrganization() | ||||||||||||||||||
| // Persist provider tokens into encrypted HttpOnly cookies (best-effort) | ||||||||||||||||||
| const { session } = data | ||||||||||||||||||
| const access = session?.provider_token ?? '' | ||||||||||||||||||
| const refresh = session?.provider_refresh_token ?? '' | ||||||||||||||||||
| // received provider tokens for cookie write | ||||||||||||||||||
| // Access token expiration: 8 hours. | ||||||||||||||||||
| // GitHub's OAuth access tokens typically expire after 8 hours. See: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#step-3-github-redirects-back-to-your-site | ||||||||||||||||||
| const EIGHT_HOURS_MS = 8 * 60 * 60 * 1000 | ||||||||||||||||||
|
Comment on lines
+27
to
+30
|
||||||||||||||||||
| // received provider tokens for cookie write | |
| const EIGHT_HOURS_MS = 8 * 60 * 60 * 1000 | |
| // received provider tokens for cookie write | |
| // Access token expiration: 8 hours. | |
| // GitHub's OAuth access tokens typically expire after 8 hours. See: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#step-3-github-redirects-back-to-your-site | |
| const EIGHT_HOURS_MS = 8 * 60 * 60 * 1000 | |
| // Refresh token expiration: 30 days. | |
| // GitHub's OAuth refresh tokens typically expire after 30 days. See: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#step-3-github-redirects-back-to-your-site |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub refresh tokens are valid for 6 months (180 days), not 30 days. This hardcoded value causes refresh tokens to be considered expired after 30 days, even though they're still valid. Why?
https://docs.github.com/ja/apps/creating-github-apps/authenticating-with-a-github-app/refreshing-user-access-tokens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this point earlier.
I intended to align with any standard HttpOnly cookie storage duration, but since web standards and common security guidelines don't specifically mention it, I'll match the lifetime of GitHub tokens instead. I'll address this in the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intended to align with any standard HttpOnly cookie storage duration.
I don't think there's a problem with that itself, but using magic numbers is not good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Access tokens and refresh tokens must be obtained during login. Since this is a critical path, we want to monitor it with Sentry without stopping the app due to errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent error handling may hide logout failures.
Both success and error outcomes are discarded without logging. While always returning
{ ok: true }might be intentional for logout UX (client should proceed even if server fails), completely suppressing errors makes debugging difficult.Consider logging errors for observability:
export async function POST() { - await fromAsyncThrowable(clearTokens)().match( - () => undefined, - () => undefined, - ) + await fromAsyncThrowable(clearTokens)()._unsafeUnwrap() return NextResponse.json({ ok: true }) }Alternatively, if you want to always succeed but still log errors:
export async function POST() { - await fromAsyncThrowable(clearTokens)().match( - () => undefined, - () => undefined, - ) + const result = await fromAsyncThrowable(clearTokens)() + if (result.isErr()) { + console.error('Failed to clear tokens during logout:', result.error) + } return NextResponse.json({ ok: true }) }📝 Committable suggestion
🤖 Prompt for AI Agents