fix(opencode): improve console login transport errors (#21350)
parent
81bdffc81c
commit
d83fe4b540
|
|
@ -1,9 +1,10 @@
|
|||
import { Cache, Clock, Duration, Effect, Layer, Option, Schema, SchemaGetter, ServiceMap } from "effect"
|
||||
import { FetchHttpClient, HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"
|
||||
import { FetchHttpClient, HttpClient, HttpClientError, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"
|
||||
|
||||
import { makeRuntime } from "@/effect/run-service"
|
||||
import { withTransientReadRetry } from "@/util/effect-http-client"
|
||||
import { AccountRepo, type AccountRow } from "./repo"
|
||||
import { normalizeServerUrl } from "./url"
|
||||
import {
|
||||
type AccountError,
|
||||
AccessToken,
|
||||
|
|
@ -12,6 +13,7 @@ import {
|
|||
Info,
|
||||
RefreshToken,
|
||||
AccountServiceError,
|
||||
AccountTransportError,
|
||||
Login,
|
||||
Org,
|
||||
OrgID,
|
||||
|
|
@ -30,6 +32,7 @@ export {
|
|||
type AccountError,
|
||||
AccountRepoError,
|
||||
AccountServiceError,
|
||||
AccountTransportError,
|
||||
AccessToken,
|
||||
RefreshToken,
|
||||
DeviceCode,
|
||||
|
|
@ -132,13 +135,30 @@ const isTokenFresh = (tokenExpiry: number | null, now: number) =>
|
|||
|
||||
const mapAccountServiceError =
|
||||
(message = "Account service operation failed") =>
|
||||
<A, E, R>(effect: Effect.Effect<A, E, R>): Effect.Effect<A, AccountServiceError, R> =>
|
||||
<A, E, R>(effect: Effect.Effect<A, E, R>): Effect.Effect<A, AccountError, R> =>
|
||||
effect.pipe(
|
||||
Effect.mapError((cause) =>
|
||||
cause instanceof AccountServiceError ? cause : new AccountServiceError({ message, cause }),
|
||||
),
|
||||
Effect.mapError((cause) => accountErrorFromCause(cause, message)),
|
||||
)
|
||||
|
||||
const accountErrorFromCause = (cause: unknown, message: string): AccountError => {
|
||||
if (cause instanceof AccountServiceError || cause instanceof AccountTransportError) {
|
||||
return cause
|
||||
}
|
||||
|
||||
if (HttpClientError.isHttpClientError(cause)) {
|
||||
switch (cause.reason._tag) {
|
||||
case "TransportError": {
|
||||
return AccountTransportError.fromHttpClientError(cause.reason)
|
||||
}
|
||||
default: {
|
||||
return new AccountServiceError({ message, cause })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return new AccountServiceError({ message, cause })
|
||||
}
|
||||
|
||||
export namespace Account {
|
||||
export interface Interface {
|
||||
readonly active: () => Effect.Effect<Option.Option<Info>, AccountError>
|
||||
|
|
@ -346,8 +366,9 @@ export namespace Account {
|
|||
})
|
||||
|
||||
const login = Effect.fn("Account.login")(function* (server: string) {
|
||||
const normalizedServer = normalizeServerUrl(server)
|
||||
const response = yield* executeEffectOk(
|
||||
HttpClientRequest.post(`${server}/auth/device/code`).pipe(
|
||||
HttpClientRequest.post(`${normalizedServer}/auth/device/code`).pipe(
|
||||
HttpClientRequest.acceptJson,
|
||||
HttpClientRequest.schemaBodyJson(ClientId)(new ClientId({ client_id: clientId })),
|
||||
),
|
||||
|
|
@ -359,8 +380,8 @@ export namespace Account {
|
|||
return new Login({
|
||||
code: parsed.device_code,
|
||||
user: parsed.user_code,
|
||||
url: `${server}${parsed.verification_uri_complete}`,
|
||||
server,
|
||||
url: `${normalizedServer}${parsed.verification_uri_complete}`,
|
||||
server: normalizedServer,
|
||||
expiry: parsed.expires_in,
|
||||
interval: parsed.interval,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { Effect, Layer, Option, Schema, ServiceMap } from "effect"
|
|||
import { Database } from "@/storage/db"
|
||||
import { AccountStateTable, AccountTable } from "./account.sql"
|
||||
import { AccessToken, AccountID, AccountRepoError, Info, OrgID, RefreshToken } from "./schema"
|
||||
import { normalizeServerUrl } from "./url"
|
||||
|
||||
export type AccountRow = (typeof AccountTable)["$inferSelect"]
|
||||
|
||||
|
|
@ -125,11 +126,13 @@ export class AccountRepo extends ServiceMap.Service<AccountRepo, AccountRepo.Ser
|
|||
|
||||
const persistAccount = Effect.fn("AccountRepo.persistAccount")((input) =>
|
||||
tx((db) => {
|
||||
const url = normalizeServerUrl(input.url)
|
||||
|
||||
db.insert(AccountTable)
|
||||
.values({
|
||||
id: input.id,
|
||||
email: input.email,
|
||||
url: input.url,
|
||||
url,
|
||||
access_token: input.accessToken,
|
||||
refresh_token: input.refreshToken,
|
||||
token_expiry: input.expiry,
|
||||
|
|
@ -138,7 +141,7 @@ export class AccountRepo extends ServiceMap.Service<AccountRepo, AccountRepo.Ser
|
|||
target: AccountTable.id,
|
||||
set: {
|
||||
email: input.email,
|
||||
url: input.url,
|
||||
url,
|
||||
access_token: input.accessToken,
|
||||
refresh_token: input.refreshToken,
|
||||
token_expiry: input.expiry,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { Schema } from "effect"
|
||||
import type * as HttpClientError from "effect/unstable/http/HttpClientError"
|
||||
|
||||
import { withStatics } from "@/util/schema"
|
||||
|
||||
|
|
@ -60,7 +61,34 @@ export class AccountServiceError extends Schema.TaggedErrorClass<AccountServiceE
|
|||
cause: Schema.optional(Schema.Defect),
|
||||
}) {}
|
||||
|
||||
export type AccountError = AccountRepoError | AccountServiceError
|
||||
export class AccountTransportError extends Schema.TaggedErrorClass<AccountTransportError>()("AccountTransportError", {
|
||||
method: Schema.String,
|
||||
url: Schema.String,
|
||||
description: Schema.optional(Schema.String),
|
||||
cause: Schema.optional(Schema.Defect),
|
||||
}) {
|
||||
static fromHttpClientError(error: HttpClientError.TransportError): AccountTransportError {
|
||||
return new AccountTransportError({
|
||||
method: error.request.method,
|
||||
url: error.request.url,
|
||||
description: error.description,
|
||||
cause: error.cause,
|
||||
})
|
||||
}
|
||||
|
||||
override get message(): string {
|
||||
return [
|
||||
`Could not reach ${this.method} ${this.url}.`,
|
||||
`This failed before the server returned an HTTP response.`,
|
||||
this.description,
|
||||
`Check your network, proxy, or VPN configuration and try again.`,
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join("\n")
|
||||
}
|
||||
}
|
||||
|
||||
export type AccountError = AccountRepoError | AccountServiceError | AccountTransportError
|
||||
|
||||
export class Login extends Schema.Class<Login>("Login")({
|
||||
code: DeviceCode,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,8 @@
|
|||
export const normalizeServerUrl = (input: string): string => {
|
||||
const url = new URL(input)
|
||||
url.search = ""
|
||||
url.hash = ""
|
||||
|
||||
const pathname = url.pathname.replace(/\/+$/, "")
|
||||
return pathname.length === 0 ? url.origin : `${url.origin}${pathname}`
|
||||
}
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
import { AccountServiceError, AccountTransportError } from "@/account"
|
||||
import { ConfigMarkdown } from "@/config/markdown"
|
||||
import { errorFormat } from "@/util/error"
|
||||
import { Config } from "../config/config"
|
||||
|
|
@ -8,6 +9,9 @@ import { UI } from "./ui"
|
|||
export function FormatError(input: unknown) {
|
||||
if (MCP.Failed.isInstance(input))
|
||||
return `MCP server "${input.data.name}" failed. Note, opencode does not support MCP authentication yet.`
|
||||
if (input instanceof AccountTransportError || input instanceof AccountServiceError) {
|
||||
return input.message
|
||||
}
|
||||
if (Provider.ModelNotFoundError.isInstance(input)) {
|
||||
const { providerID, modelID, suggestions } = input.data
|
||||
return [
|
||||
|
|
|
|||
|
|
@ -56,6 +56,32 @@ it.live("persistAccount inserts and getRow retrieves", () =>
|
|||
}),
|
||||
)
|
||||
|
||||
it.live("persistAccount normalizes trailing slashes in stored server URLs", () =>
|
||||
Effect.gen(function* () {
|
||||
const id = AccountID.make("user-1")
|
||||
|
||||
yield* AccountRepo.use((r) =>
|
||||
r.persistAccount({
|
||||
id,
|
||||
email: "test@example.com",
|
||||
url: "https://control.example.com/",
|
||||
accessToken: AccessToken.make("at_123"),
|
||||
refreshToken: RefreshToken.make("rt_456"),
|
||||
expiry: Date.now() + 3600_000,
|
||||
orgID: Option.none(),
|
||||
}),
|
||||
)
|
||||
|
||||
const row = yield* AccountRepo.use((r) => r.getRow(id))
|
||||
const active = yield* AccountRepo.use((r) => r.active())
|
||||
const list = yield* AccountRepo.use((r) => r.list())
|
||||
|
||||
expect(Option.getOrThrow(row).url).toBe("https://control.example.com")
|
||||
expect(Option.getOrThrow(active).url).toBe("https://control.example.com")
|
||||
expect(list[0]?.url).toBe("https://control.example.com")
|
||||
}),
|
||||
)
|
||||
|
||||
it.live("persistAccount sets the active account and org", () =>
|
||||
Effect.gen(function* () {
|
||||
const id1 = AccountID.make("user-1")
|
||||
|
|
|
|||
|
|
@ -1,10 +1,20 @@
|
|||
import { expect } from "bun:test"
|
||||
import { Duration, Effect, Layer, Option, Schema } from "effect"
|
||||
import { HttpClient, HttpClientResponse } from "effect/unstable/http"
|
||||
import { HttpClient, HttpClientError, HttpClientResponse } from "effect/unstable/http"
|
||||
|
||||
import { AccountRepo } from "../../src/account/repo"
|
||||
import { Account } from "../../src/account"
|
||||
import { AccessToken, AccountID, DeviceCode, Login, Org, OrgID, RefreshToken, UserCode } from "../../src/account/schema"
|
||||
import {
|
||||
AccessToken,
|
||||
AccountID,
|
||||
AccountTransportError,
|
||||
DeviceCode,
|
||||
Login,
|
||||
Org,
|
||||
OrgID,
|
||||
RefreshToken,
|
||||
UserCode,
|
||||
} from "../../src/account/schema"
|
||||
import { Database } from "../../src/storage/db"
|
||||
import { testEffect } from "../lib/effect"
|
||||
|
||||
|
|
@ -57,6 +67,57 @@ const deviceTokenClient = (body: unknown, status = 400) =>
|
|||
const poll = (body: unknown, status = 400) =>
|
||||
Account.Service.use((s) => s.poll(login())).pipe(Effect.provide(live(deviceTokenClient(body, status))))
|
||||
|
||||
it.live("login normalizes trailing slashes in the provided server URL", () =>
|
||||
Effect.gen(function* () {
|
||||
const seen: Array<string> = []
|
||||
const client = HttpClient.make((req) =>
|
||||
Effect.gen(function* () {
|
||||
seen.push(`${req.method} ${req.url}`)
|
||||
|
||||
if (req.url === "https://one.example.com/auth/device/code") {
|
||||
return json(req, {
|
||||
device_code: "device-code",
|
||||
user_code: "user-code",
|
||||
verification_uri_complete: "/device?user_code=user-code",
|
||||
expires_in: 600,
|
||||
interval: 5,
|
||||
})
|
||||
}
|
||||
|
||||
return json(req, {}, 404)
|
||||
}),
|
||||
)
|
||||
|
||||
const result = yield* Account.Service.use((s) => s.login("https://one.example.com/")).pipe(Effect.provide(live(client)))
|
||||
|
||||
expect(seen).toEqual(["POST https://one.example.com/auth/device/code"])
|
||||
expect(result.server).toBe("https://one.example.com")
|
||||
expect(result.url).toBe("https://one.example.com/device?user_code=user-code")
|
||||
}),
|
||||
)
|
||||
|
||||
it.live("login maps transport failures to account transport errors", () =>
|
||||
Effect.gen(function* () {
|
||||
const client = HttpClient.make((req) =>
|
||||
Effect.fail(
|
||||
new HttpClientError.HttpClientError({
|
||||
reason: new HttpClientError.TransportError({ request: req }),
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
const error = yield* Effect.flip(
|
||||
Account.Service.use((s) => s.login("https://one.example.com")).pipe(Effect.provide(live(client))),
|
||||
)
|
||||
|
||||
expect(error).toBeInstanceOf(AccountTransportError)
|
||||
if (error instanceof AccountTransportError) {
|
||||
expect(error.method).toBe("POST")
|
||||
expect(error.url).toBe("https://one.example.com/auth/device/code")
|
||||
}
|
||||
}),
|
||||
)
|
||||
|
||||
it.live("orgsByAccount groups orgs per account", () =>
|
||||
Effect.gen(function* () {
|
||||
yield* AccountRepo.use((r) =>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
import { describe, expect, test } from "bun:test"
|
||||
import { AccountTransportError } from "../../src/account/schema"
|
||||
import { FormatError } from "../../src/cli/error"
|
||||
|
||||
describe("cli.error", () => {
|
||||
test("formats account transport errors clearly", () => {
|
||||
const error = new AccountTransportError({
|
||||
method: "POST",
|
||||
url: "https://console.opencode.ai/auth/device/code",
|
||||
})
|
||||
|
||||
const formatted = FormatError(error)
|
||||
|
||||
expect(formatted).toContain("Could not reach POST https://console.opencode.ai/auth/device/code.")
|
||||
expect(formatted).toContain("This failed before the server returned an HTTP response.")
|
||||
expect(formatted).toContain("Check your network, proxy, or VPN configuration and try again.")
|
||||
})
|
||||
})
|
||||
Loading…
Reference in New Issue