Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 134
fix: defensive null guards in tool formatters and DuckDB concurrent access retry#571
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
6157ecd89981cd5dbb1bf4af4119d2cdfe8File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -17,10 +17,24 @@ export async function connect(config: ConnectionConfig): Promise<Connector> { | ||
| let db: any | ||
| let connection: any | ||
| // altimate_change start — improve DuckDB error messages | ||
| function wrapDuckDBError(err: Error): Error { | ||
| const msg = err.message || String(err) | ||
| if (msg.toLowerCase().includes("locked") || msg.includes("SQLITE_BUSY") || msg.includes("DUCKDB_LOCKED")) { | ||
| return new Error( | ||
| `Database "${dbPath}" is locked by another process. ` + | ||
| `DuckDB does not support concurrent write access. ` + | ||
| `Close other connections to this file and try again.`, | ||
| ) | ||
| } | ||
| return err | ||
| } | ||
| // altimate_change end | ||
| function query(sql: string): Promise<any[]> { | ||
| return new Promise((resolve, reject) => { | ||
| connection.all(sql, (err: Error | null, rows: any[]) => { | ||
| if (err) reject(err) | ||
| if (err) reject(wrapDuckDBError(err)) | ||
| else resolve(rows ?? []) | ||
| }) | ||
| }) | ||
| @@ -29,33 +43,65 @@ export async function connect(config: ConnectionConfig): Promise<Connector> { | ||
| function queryWithParams(sql: string, params: any[]): Promise<any[]> { | ||
| return new Promise((resolve, reject) => { | ||
| connection.all(sql, ...params, (err: Error | null, rows: any[]) => { | ||
| if (err) reject(err) | ||
| if (err) reject(wrapDuckDBError(err)) | ||
| else resolve(rows ?? []) | ||
| }) | ||
| }) | ||
| } | ||
| return { | ||
| async connect() { | ||
| db = await new Promise<any>((resolve, reject) => { | ||
| let resolved = false | ||
| const instance = new duckdb.Database( | ||
| dbPath, | ||
| (err: Error | null) => { | ||
| if (resolved) return // Already resolved via timeout | ||
| resolved = true | ||
| if (err) reject(err) | ||
| else resolve(instance) | ||
| }, | ||
| ) | ||
| // Bun: native callback may not fire; fall back after 2s | ||
| setTimeout(() => { | ||
| if (!resolved) { | ||
| resolved = true | ||
| resolve(instance) | ||
| // altimate_change start — retry with read-only on lock errors | ||
| const tryConnect = (accessMode?: string): Promise<any> => | ||
| new Promise<any>((resolve, reject) => { | ||
| let resolved = false | ||
| let timeout: ReturnType<typeof setTimeout> | undefined | ||
| const opts = accessMode ? { access_mode: accessMode } : undefined | ||
| const instance = new duckdb.Database( | ||
| dbPath, | ||
| opts, | ||
| (err: Error | null) => { | ||
| if (resolved) { if (instance && typeof instance.close === "function") instance.close(); return } | ||
| resolved = true | ||
| if (timeout) clearTimeout(timeout) | ||
| if (err) { | ||
| const msg = err.message || String(err) | ||
| if (msg.toLowerCase().includes("locked") || msg.includes("SQLITE_BUSY") || msg.includes("DUCKDB_LOCKED")) { | ||
| reject(new Error("DUCKDB_LOCKED")) | ||
| } else { | ||
| reject(err) | ||
| } | ||
| } else { | ||
| resolve(instance) | ||
| } | ||
| }, | ||
| ) | ||
| // Bun: native callback may not fire; fall back after 2s | ||
| timeout = setTimeout(() => { | ||
| if (!resolved) { | ||
| resolved = true | ||
| reject(new Error(`Timed out opening DuckDB database "${dbPath}"`)) | ||
| } | ||
| }, 2000) | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }) | ||
| try { | ||
| db = await tryConnect() | ||
| } catch (err: any) { | ||
| if (err.message === "DUCKDB_LOCKED" && dbPath !== ":memory:") { | ||
| // Retry in read-only mode — allows concurrent reads | ||
| try { | ||
| db = await tryConnect("READ_ONLY") | ||
| } catch (retryErr) { | ||
| throw wrapDuckDBError( | ||
| retryErr instanceof Error ? retryErr : new Error(String(retryErr)), | ||
| ) | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| }, 2000) | ||
| }) | ||
| } else { | ||
| throw err | ||
| } | ||
| } | ||
| // altimate_change end | ||
| connection = db.connect() | ||
| }, | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -38,8 +38,8 @@ export const SqlAnalyzeTool = Tool.define("sql_analyze", { | ||
| // there's an actual error (e.g. parse failure). | ||
| const isRealFailure = !!result.error | ||
| // altimate_change start — sql quality findings for telemetry | ||
| const findings: Telemetry.Finding[] = result.issues.map((issue) => ({ | ||
| category: issue.rule ?? issue.type, | ||
| const findings: Telemetry.Finding[] = (result.issues ?? []).map((issue) => ({ | ||
| category: issue.rule ?? issue.type ?? "analysis_issue", | ||
| })) | ||
coderabbitai[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // altimate_change end | ||
| @@ -56,7 +56,7 @@ export const SqlAnalyzeTool = Tool.define("sql_analyze", { | ||
| } | ||
| // altimate_change end | ||
| return { | ||
| title: `Analyze: ${result.error ? "ERROR" : `${result.issue_count} issue${result.issue_count !== 1 ? "s" : ""}`} [${result.confidence}]`, | ||
| title: `Analyze: ${result.error ? "ERROR" : `${result.issue_count ?? 0} issue${(result.issue_count ?? 0) !== 1 ? "s" : ""}`} [${result.confidence ?? "unknown"}]`, | ||
| metadata: { | ||
| success: !isRealFailure, | ||
| issueCount: result.issue_count, | ||
| @@ -91,24 +91,27 @@ function formatAnalysis(result: SqlAnalyzeResult): string { | ||
| return `Analysis failed: ${result.error}` | ||
| } | ||
| if (result.issues.length === 0) { | ||
| const issues = result.issues ?? [] | ||
| if (issues.length === 0) { | ||
| return "No anti-patterns or issues detected." | ||
| } | ||
| const issueCount = result.issue_count ?? issues.length | ||
| const lines: string[] = [ | ||
| `Found ${result.issue_count} issue${result.issue_count !== 1 ? "s" : ""} (confidence: ${result.confidence}):`, | ||
| `Found ${issueCount} issue${issueCount !== 1 ? "s" : ""} (confidence: ${result.confidence ?? "unknown"}):`, | ||
| ] | ||
| if (result.confidence_factors.length > 0) { | ||
| lines.push(` Note: ${result.confidence_factors.join("; ")}`) | ||
| const factors = result.confidence_factors ?? [] | ||
| if (factors.length > 0) { | ||
| lines.push(` Note: ${factors.join("; ")}`) | ||
| } | ||
| lines.push("") | ||
| for (const issue of result.issues) { | ||
| for (const issue of issues) { | ||
| const loc = issue.location ? ` — ${issue.location}` : "" | ||
| const conf = issue.confidence !== "high" ? ` [${issue.confidence} confidence]` : "" | ||
| lines.push(` [${issue.severity.toUpperCase()}] ${issue.type}${conf}`) | ||
| lines.push(` ${issue.message}${loc}`) | ||
| lines.push(` → ${issue.recommendation}`) | ||
| const conf = issue.confidence !== "high" ? ` [${issue.confidence ?? "unknown"} confidence]` : "" | ||
| lines.push(` [${String(issue.severity ?? "unknown").toUpperCase()}] ${issue.type ?? "unknown"}${conf}`) | ||
| lines.push(` ${issue.message ?? ""}${loc}`) | ||
| lines.push(` → ${issue.recommendation ?? ""}`) | ||
| lines.push("") | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.