Skip to content

Commit ffe03b4

Browse files
anandgupta42claude
andauthored
fix: improve telemetry error classification, warehouse validation, and webfetch error context (#566)
Driven by App Insights analysis of `altimate-code-os` showing: - 89 `core_failure` events/day with blind `"unknown error"` message - 694 webfetch 404s classified as `error_class: "unknown"` - 41 SQL failures from cryptic SASL/SCRAM password errors - 4 ClickHouse attempts hitting generic "unsupported" error **Telemetry** — add `http_error` class, expand connection/validation/permission patterns, redact sensitive keys in `computeInputSignature()` **Tool errors** — replace `"unknown error"` with structured `soft_failure:{tool}:no_error_metadata`, use `maskArgs()` for objects **Warehouse** — validate password type in postgres/redshift drivers and registry, harden `statement_timeout`, add `KNOWN_UNSUPPORTED` hints **Webfetch** — include hostname in HTTP error messages **Tests** — 20+ new `classifyError` assertions including `http_error`, expanded patterns, and priority ordering tests Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0226200 commit ffe03b4

File tree

7 files changed

+162
-15
lines changed

7 files changed

+162
-15
lines changed

packages/drivers/src/postgres.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
2222
if (config.connection_string) {
2323
poolConfig.connectionString = config.connection_string
2424
} else {
25+
// Validate required credentials before connecting to avoid cryptic
26+
// SASL/SCRAM errors from the pg driver when password is missing
27+
if (config.password != null && typeof config.password !== "string") {
28+
throw new Error(
29+
"PostgreSQL password must be a string. Check your warehouse configuration.",
30+
)
31+
}
2532
poolConfig.host = config.host ?? "127.0.0.1"
2633
poolConfig.port = config.port ?? 5432
2734
poolConfig.database = config.database ?? "postgres"
@@ -43,9 +50,10 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
4350
const client = await pool.connect()
4451
try {
4552
if (config.statement_timeout) {
46-
await client.query(
47-
`SET statement_timeout = '${Number(config.statement_timeout)}ms'`,
48-
)
53+
const timeoutMs = Number(config.statement_timeout)
54+
if (Number.isFinite(timeoutMs) && timeoutMs > 0) {
55+
await client.query(`SET statement_timeout TO ${Math.round(timeoutMs)}`)
56+
}
4957
}
5058

5159
let query = sql

packages/drivers/src/redshift.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
2525
if (config.connection_string) {
2626
poolConfig.connectionString = config.connection_string
2727
} else {
28+
// Validate password type to prevent cryptic SASL/SCRAM errors
29+
if (config.password != null && typeof config.password !== "string") {
30+
throw new Error(
31+
"Redshift password must be a string. Check your warehouse configuration.",
32+
)
33+
}
2834
poolConfig.host = config.host ?? "127.0.0.1"
2935
poolConfig.port = config.port ?? 5439 // Redshift default
3036
poolConfig.database = config.database ?? "dev"

packages/opencode/src/altimate/native/connections/registry.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,18 @@ const DRIVER_MAP: Record<string, string> = {
133133
async function createConnector(name: string, config: ConnectionConfig): Promise<Connector> {
134134
const driverPath = DRIVER_MAP[config.type.toLowerCase()]
135135
if (!driverPath) {
136+
// altimate_change start — friendlier error for known-but-unsupported databases
137+
const KNOWN_UNSUPPORTED: Record<string, string> = {
138+
clickhouse: "ClickHouse is not yet supported. Use the bash tool with `clickhouse-client` or `curl` to query ClickHouse directly.",
139+
cassandra: "Cassandra is not yet supported. Use the bash tool with `cqlsh` to query Cassandra directly.",
140+
cockroachdb: "CockroachDB is not yet supported. It is PostgreSQL-compatible — try type: postgres instead.",
141+
timescaledb: "TimescaleDB is a PostgreSQL extension — use type: postgres instead.",
142+
}
143+
const hint = KNOWN_UNSUPPORTED[config.type.toLowerCase()]
144+
if (hint) {
145+
throw new Error(hint)
146+
}
147+
// altimate_change end
136148
throw new Error(`Unsupported database type: ${config.type}. Supported: ${Object.keys(DRIVER_MAP).join(", ")}`)
137149
}
138150

@@ -143,6 +155,22 @@ async function createConnector(name: string, config: ConnectionConfig): Promise<
143155
// Resolve credentials from keychain
144156
resolvedConfig = await resolveConfig(name, resolvedConfig)
145157

158+
// altimate_change start — validate password is a string for drivers that require it
159+
// Prevents cryptic SASL/SCRAM errors from database drivers
160+
const PASSWORD_DRIVERS = new Set(["postgres", "postgresql", "redshift", "mysql", "mariadb", "sqlserver", "mssql", "oracle", "snowflake"])
161+
if (
162+
PASSWORD_DRIVERS.has(resolvedConfig.type.toLowerCase()) &&
163+
!resolvedConfig.connection_string &&
164+
resolvedConfig.password != null &&
165+
typeof resolvedConfig.password !== "string"
166+
) {
167+
throw new Error(
168+
`Database password must be a string for ${resolvedConfig.type}. ` +
169+
"Check your warehouse configuration or re-add the connection with warehouse.add.",
170+
)
171+
}
172+
// altimate_change end
173+
146174
// Handle SSH tunnel
147175
const sshConfig = extractSshConfig(resolvedConfig)
148176
if (sshConfig) {

packages/opencode/src/altimate/telemetry/index.ts

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ export namespace Telemetry {
403403
session_id: string
404404
tool_name: string
405405
tool_category: string
406-
error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "unknown"
406+
error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "http_error" | "unknown"
407407
error_message: string
408408
input_signature: string
409409
masked_args?: string
@@ -425,20 +425,57 @@ export namespace Telemetry {
425425
}
426426
// altimate_change end
427427

428+
// altimate_change start — expanded error classification patterns for better triage
429+
// Order matters: earlier patterns take priority. Use specific phrases, not
430+
// single words, to avoid false positives (e.g., "connection refused" not "connection").
428431
const ERROR_PATTERNS: Array<{
429432
class: Telemetry.Event & { type: "core_failure" } extends { error_class: infer C } ? C : never
430433
keywords: string[]
431434
}> = [
432435
{ class: "parse_error", keywords: ["parse", "syntax", "binder", "unexpected token", "sqlglot"] },
433436
{
434437
class: "connection",
435-
keywords: ["econnrefused", "connection", "socket", "enotfound", "econnreset"],
438+
keywords: [
439+
"econnrefused",
440+
"enotfound",
441+
"econnreset",
442+
"connection refused",
443+
"connection reset",
444+
"connection closed",
445+
"connect failed",
446+
"connect etimedout",
447+
"socket hang up",
448+
"sasl",
449+
"scram",
450+
"password must be",
451+
"driver not installed",
452+
"not found. available:",
453+
"no warehouse configured",
454+
"unsupported database type",
455+
],
436456
},
437457
{ class: "timeout", keywords: ["timeout", "etimedout", "bridge timeout", "timed out"] },
438-
{ class: "permission", keywords: ["permission", "denied", "unauthorized", "forbidden"] },
439-
{ class: "validation", keywords: ["invalid params", "invalid", "missing", "required"] },
458+
{ class: "permission", keywords: ["permission", "access denied", "permission denied", "unauthorized", "forbidden", "authentication"] },
459+
{
460+
class: "validation",
461+
keywords: [
462+
"invalid params",
463+
"invalid",
464+
"missing",
465+
"required",
466+
"must read file",
467+
"has been modified since",
468+
"does not exist",
469+
"before overwriting",
470+
],
471+
},
440472
{ class: "internal", keywords: ["internal", "assertion"] },
473+
{
474+
class: "http_error",
475+
keywords: ["status code: 4", "status code: 5", "request failed with status"],
476+
},
441477
]
478+
// altimate_change end
442479

443480
export function classifyError(
444481
message: string,
@@ -453,6 +490,12 @@ export namespace Telemetry {
453490
export function computeInputSignature(args: Record<string, unknown>): string {
454491
const sig: Record<string, string> = {}
455492
for (const [k, v] of Object.entries(args)) {
493+
// altimate_change start — redact sensitive keys in input signatures
494+
if (isSensitiveKey(k)) {
495+
sig[k] = "****"
496+
continue
497+
}
498+
// altimate_change end
456499
if (v === null || v === undefined) {
457500
sig[k] = "null"
458501
} else if (typeof v === "string") {

packages/opencode/src/tool/tool.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,25 @@ export namespace Tool {
145145
}
146146
if (isSoftFailure) {
147147
// prettier-ignore
148-
const errorMsg =
149-
typeof result.metadata?.error === "string"
150-
? result.metadata.error
151-
: "unknown error"
148+
// altimate_change start — propagate non-string error metadata to avoid blind "unknown error"
149+
const rawError = result.metadata?.error
150+
let errorMsg: string
151+
if (typeof rawError === "string") {
152+
errorMsg = rawError
153+
} else if (rawError != null) {
154+
if (typeof rawError === "object") {
155+
try {
156+
errorMsg = Telemetry.maskArgs(rawError as Record<string, unknown>)
157+
} catch {
158+
errorMsg = `soft_failure:${id}:unserializable_error`
159+
}
160+
} else {
161+
errorMsg = String(rawError)
162+
}
163+
} else {
164+
errorMsg = `soft_failure:${id}:no_error_metadata`
165+
}
166+
// altimate_change end
152167
const maskedErrorMsg = Telemetry.maskString(errorMsg).slice(0, 500)
153168
Telemetry.track({
154169
type: "core_failure",

packages/opencode/src/tool/webfetch.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,17 @@ export const WebFetchTool = Tool.define("webfetch", {
8484
}
8585

8686
if (!response.ok) {
87-
throw new Error(`Request failed with status code: ${response.status}`)
87+
// altimate_change start — include URL domain in error for easier triage
88+
let domain: string
89+
try {
90+
domain = new URL(params.url).hostname
91+
} catch {
92+
domain = params.url.slice(0, 60)
93+
}
94+
throw new Error(
95+
`Request failed with status code: ${response.status} (${domain})`,
96+
)
97+
// altimate_change end
8898
}
8999

90100
// Check content length

packages/opencode/test/telemetry/telemetry.test.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,6 +1538,16 @@ describe("telemetry.classifyError", () => {
15381538
expect(Telemetry.classifyError("Socket hang up")).toBe("connection")
15391539
expect(Telemetry.classifyError("ENOTFOUND db.example.com")).toBe("connection")
15401540
expect(Telemetry.classifyError("ECONNRESET")).toBe("connection")
1541+
// altimate_change start — expanded connection patterns
1542+
expect(Telemetry.classifyError("SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string")).toBe("connection")
1543+
expect(Telemetry.classifyError("password must be a string")).toBe("connection")
1544+
expect(Telemetry.classifyError("PostgreSQL driver not installed. Run: npm install pg")).toBe("connection")
1545+
expect(Telemetry.classifyError("Error: Connection mydb not found. Available: (none)")).toBe("connection")
1546+
expect(Telemetry.classifyError("No warehouse configured. Use warehouse.add")).toBe("connection")
1547+
expect(Telemetry.classifyError("Unsupported database type: clickhouse")).toBe("connection")
1548+
expect(Telemetry.classifyError("Connection reset by peer")).toBe("connection")
1549+
expect(Telemetry.classifyError("Connection closed unexpectedly")).toBe("connection")
1550+
// altimate_change end
15411551
})
15421552

15431553
test("classifies timeout errors", () => {
@@ -1552,20 +1562,47 @@ describe("telemetry.classifyError", () => {
15521562
expect(Telemetry.classifyError("Invalid dialect specified")).toBe("validation")
15531563
expect(Telemetry.classifyError("Missing required field")).toBe("validation")
15541564
expect(Telemetry.classifyError("Required parameter 'query' not provided")).toBe("validation")
1565+
// altimate_change start — expanded validation patterns
1566+
expect(Telemetry.classifyError("You must read file /path/to/file before overwriting it")).toBe("validation")
1567+
expect(Telemetry.classifyError("File has been modified since it was last read")).toBe("validation")
1568+
expect(Telemetry.classifyError("error: column foo does not exist")).toBe("validation")
1569+
expect(Telemetry.classifyError("You must read file before overwriting it. Use the Read tool first")).toBe("validation")
1570+
// altimate_change end
15551571
})
15561572

15571573
test("classifies permission errors", () => {
15581574
expect(Telemetry.classifyError("Permission denied on table")).toBe("permission")
15591575
expect(Telemetry.classifyError("Access denied for user")).toBe("permission")
15601576
expect(Telemetry.classifyError("Unauthorized access to resource")).toBe("permission")
15611577
expect(Telemetry.classifyError("403 Forbidden")).toBe("permission")
1578+
// altimate_change start — authentication classified as permission
1579+
expect(Telemetry.classifyError("Authentication failed for user admin")).toBe("permission")
1580+
// altimate_change end
15621581
})
15631582

15641583
test("classifies internal errors", () => {
15651584
expect(Telemetry.classifyError("Internal server error")).toBe("internal")
15661585
expect(Telemetry.classifyError("Assertion failed: x > 0")).toBe("internal")
15671586
})
15681587

1588+
// altimate_change start — http_error class and priority ordering tests
1589+
test("classifies http errors", () => {
1590+
expect(Telemetry.classifyError("Request failed with status code: 404 (example.com)")).toBe("http_error")
1591+
expect(Telemetry.classifyError("Request failed with status code: 500")).toBe("http_error")
1592+
expect(Telemetry.classifyError("status code: 403")).toBe("http_error")
1593+
expect(Telemetry.classifyError("Request failed with status")).toBe("http_error")
1594+
})
1595+
1596+
test("priority ordering: earlier patterns win over later ones", () => {
1597+
// SASL is connection, even though "authentication" is in permission
1598+
expect(Telemetry.classifyError("SASL authentication failed")).toBe("connection")
1599+
// parse_error wins over validation for "invalid syntax"
1600+
expect(Telemetry.classifyError("parse error: invalid syntax")).toBe("parse_error")
1601+
// permission ("forbidden") wins over http_error ("status code: 4")
1602+
expect(Telemetry.classifyError("403 Forbidden, status code: 403")).toBe("permission")
1603+
})
1604+
// altimate_change end
1605+
15691606
test("returns unknown for unrecognized errors", () => {
15701607
expect(Telemetry.classifyError("Something went wrong")).toBe("unknown")
15711608
expect(Telemetry.classifyError("")).toBe("unknown")
@@ -1634,11 +1671,11 @@ describe("telemetry.computeInputSignature", () => {
16341671
expect(sig).not.toContain("sk-abc123")
16351672
expect(sig).not.toContain("SELECT")
16361673
expect(sig).not.toContain("admin@example.com")
1637-
// Only type descriptors appear as values
1674+
// Only type descriptors appear as values; sensitive keys are fully redacted
16381675
const parsed = JSON.parse(sig)
16391676
expect(parsed.sql).toBe("string:60")
1640-
expect(parsed.secret).toBe("string:16")
1641-
expect(parsed.api_key).toBe("string:9")
1677+
expect(parsed.secret).toBe("****")
1678+
expect(parsed.api_key).toBe("****")
16421679
})
16431680

16441681
test("truncates output at 1000 chars with valid JSON", () => {

0 commit comments

Comments
 (0)