refactor: implement consistent error handling strategy
Error handling helpers in utils.nim:
- logError/logWarn: consistent stderr format
- wrapError template: catch and re-raise with context
db.nim:
- openBusDb: wrap with DbError and path context
- poll: handle malformed JSON gracefully with warning
- tryClaim: log failures instead of silently swallowing
git.nim:
- createWorktree/rebaseOnIntegration: warn on fetch failure
- removeWorktree/removeBranch: log cleanup failures
- getBranchStatus: log parseInt failures
context.nim:
- readContext/findContext: add path context to parse errors
- writeContext: wrap IOError with path
types.nim:
- Extract ContextDateFormat constant
- Add context to date parse errors
Closes: skills-05ah, skills-xcl, skills-266, skills-8xv, skills-8vdo,
skills-tdfm, skills-koes, skills-8bi, skills-2wjp, skills-3uv9, skills-xgh0
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
5121bbb008
commit
09b76605c2
|
|
@ -8,7 +8,10 @@ import ./types
|
|||
proc writeContext*(worktree: string, ctx: WorkerContext) =
|
||||
## Write context file to worktree
|
||||
let path = worktree / ContextFileName
|
||||
writeFile(path, $ctx.toJson())
|
||||
try:
|
||||
writeFile(path, $ctx.toJson())
|
||||
except IOError as e:
|
||||
raise newException(IOError, &"writeContext({path}): {e.msg}")
|
||||
|
||||
proc readContext*(worktree: string = ""): WorkerContext =
|
||||
## Read context from worktree. If empty, uses current directory.
|
||||
|
|
@ -18,9 +21,16 @@ proc readContext*(worktree: string = ""): WorkerContext =
|
|||
if not fileExists(path):
|
||||
raise newException(IOError, &"Context file not found: {path}")
|
||||
|
||||
let content = readFile(path)
|
||||
let j = parseJson(content)
|
||||
return WorkerContext.fromJson(j)
|
||||
try:
|
||||
let content = readFile(path)
|
||||
let j = parseJson(content)
|
||||
return WorkerContext.fromJson(j)
|
||||
except JsonParsingError as e:
|
||||
raise newException(IOError, &"readContext({path}): invalid JSON: {e.msg}")
|
||||
except KeyError as e:
|
||||
raise newException(IOError, &"readContext({path}): missing field: {e.msg}")
|
||||
except TimeParseError as e:
|
||||
raise newException(IOError, &"readContext({path}): invalid date format: {e.msg}")
|
||||
|
||||
proc findContext*(): WorkerContext =
|
||||
## Find context by walking up directory tree
|
||||
|
|
@ -28,9 +38,16 @@ proc findContext*(): WorkerContext =
|
|||
while dir != "" and dir != "/":
|
||||
let path = dir / ContextFileName
|
||||
if fileExists(path):
|
||||
let content = readFile(path)
|
||||
let j = parseJson(content)
|
||||
return WorkerContext.fromJson(j)
|
||||
try:
|
||||
let content = readFile(path)
|
||||
let j = parseJson(content)
|
||||
return WorkerContext.fromJson(j)
|
||||
except JsonParsingError as e:
|
||||
raise newException(IOError, &"findContext({path}): invalid JSON: {e.msg}")
|
||||
except KeyError as e:
|
||||
raise newException(IOError, &"findContext({path}): missing field: {e.msg}")
|
||||
except TimeParseError as e:
|
||||
raise newException(IOError, &"findContext({path}): invalid date format: {e.msg}")
|
||||
dir = parentDir(dir)
|
||||
|
||||
raise newException(IOError, "No .worker-ctx.json found in directory tree")
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@
|
|||
import std/[os, json, options, sysrand, strutils]
|
||||
import tiny_sqlite
|
||||
import ./types
|
||||
import ./utils
|
||||
|
||||
# Helper for generating unique IDs
|
||||
proc genOid*(): string =
|
||||
|
|
@ -100,13 +101,16 @@ proc initSchema*(db: DbConn) =
|
|||
|
||||
proc openBusDb*(dbPath: string = BusDbPath): DbConn =
|
||||
## Open database with required PRAGMAs. One connection per thread.
|
||||
createDir(parentDir(dbPath))
|
||||
result = openDatabase(dbPath)
|
||||
result.exec("PRAGMA busy_timeout = " & $BusyTimeoutMs)
|
||||
result.exec("PRAGMA foreign_keys = ON")
|
||||
result.exec("PRAGMA journal_mode = WAL")
|
||||
result.exec("PRAGMA synchronous = NORMAL")
|
||||
initSchema(result)
|
||||
try:
|
||||
createDir(parentDir(dbPath))
|
||||
result = openDatabase(dbPath)
|
||||
result.exec("PRAGMA busy_timeout = " & $BusyTimeoutMs)
|
||||
result.exec("PRAGMA foreign_keys = ON")
|
||||
result.exec("PRAGMA journal_mode = WAL")
|
||||
result.exec("PRAGMA synchronous = NORMAL")
|
||||
initSchema(result)
|
||||
except CatchableError as e:
|
||||
raise newException(DbError, "openBusDb(" & dbPath & "): " & e.msg)
|
||||
|
||||
proc optStr*(s: string): Option[string] =
|
||||
## Convert empty string to none
|
||||
|
|
@ -177,7 +181,12 @@ proc poll*(db: DbConn, agentId: string, limit: int = 100): seq[Message] =
|
|||
if row[7].kind != sqliteNull:
|
||||
msg.inReplyTo = some(row[7].fromDbValue(string))
|
||||
if row[8].kind != sqliteNull:
|
||||
msg.payload = some(parseJson(row[8].fromDbValue(string)))
|
||||
let payloadStr = row[8].fromDbValue(string)
|
||||
try:
|
||||
msg.payload = some(parseJson(payloadStr))
|
||||
except JsonParsingError as e:
|
||||
logWarn("poll", "malformed JSON in message " & msg.id & ": " & e.msg)
|
||||
# Skip payload but continue processing message
|
||||
if row[9].kind != sqliteNull:
|
||||
msg.payloadRef = some(row[9].fromDbValue(string))
|
||||
result.add(msg)
|
||||
|
|
@ -246,7 +255,8 @@ proc tryClaim*(db: DbConn, taskId, agentId: string): bool =
|
|||
|
||||
db.exec("COMMIT")
|
||||
return true
|
||||
except CatchableError:
|
||||
except CatchableError as e:
|
||||
logWarn("tryClaim", "failed for task " & taskId & ": " & e.msg)
|
||||
db.exec("ROLLBACK")
|
||||
return false
|
||||
|
||||
|
|
|
|||
|
|
@ -37,8 +37,10 @@ proc createWorktree*(taskId: string, fromBranch: string = "origin/integration"):
|
|||
let branch = branchName(taskId)
|
||||
let worktree = worktreePath(taskId)
|
||||
|
||||
# Fetch latest
|
||||
discard runGit("fetch", "origin")
|
||||
# Fetch latest (warn but continue on failure - may be offline)
|
||||
let (fetchOut, fetchCode) = runGit("fetch", "origin")
|
||||
if fetchCode != 0:
|
||||
logWarn("createWorktree", "fetch failed, continuing with local state: " & fetchOut.strip())
|
||||
|
||||
# Create branch from base
|
||||
discard runGitCheck("branch", branch, fromBranch)
|
||||
|
|
@ -53,18 +55,26 @@ proc removeWorktree*(taskId: string) =
|
|||
## Remove a worktree
|
||||
let worktree = worktreePath(taskId)
|
||||
if dirExists(worktree):
|
||||
discard runGit("worktree", "remove", "--force", worktree)
|
||||
let (output, code) = runGit("worktree", "remove", "--force", worktree)
|
||||
if code != 0:
|
||||
logWarn("removeWorktree", "failed for " & taskId & ": " & output.strip())
|
||||
|
||||
proc removeBranch*(taskId: string, remote: bool = true) =
|
||||
## Remove feature branch
|
||||
let branch = branchName(taskId)
|
||||
discard runGit("branch", "-D", branch)
|
||||
let (localOut, localCode) = runGit("branch", "-D", branch)
|
||||
if localCode != 0:
|
||||
logWarn("removeBranch", "local delete failed for " & branch & ": " & localOut.strip())
|
||||
if remote:
|
||||
discard runGit("push", "origin", "--delete", branch)
|
||||
let (remoteOut, remoteCode) = runGit("push", "origin", "--delete", branch)
|
||||
if remoteCode != 0:
|
||||
logWarn("removeBranch", "remote delete failed for " & branch & ": " & remoteOut.strip())
|
||||
|
||||
proc rebaseOnIntegration*(worktree: string): bool =
|
||||
## Rebase worktree on integration branch. Returns false on conflict.
|
||||
discard runGit("fetch", "origin", workDir = worktree)
|
||||
let (fetchOut, fetchCode) = runGit("fetch", "origin", workDir = worktree)
|
||||
if fetchCode != 0:
|
||||
logWarn("rebaseOnIntegration", "fetch failed, using local state: " & fetchOut.strip())
|
||||
let (output, code) = runGit("rebase", "origin/integration", workDir = worktree)
|
||||
|
||||
if code != 0:
|
||||
|
|
@ -140,5 +150,5 @@ proc getBranchStatus*(worktree: string): tuple[ahead, behind: int] =
|
|||
try:
|
||||
result.behind = parseInt(parts[0])
|
||||
result.ahead = parseInt(parts[1])
|
||||
except ValueError:
|
||||
discard
|
||||
except ValueError as e:
|
||||
logWarn("getBranchStatus", "failed to parse rev-list output '" & output.strip() & "': " & e.msg)
|
||||
|
|
|
|||
|
|
@ -102,12 +102,21 @@ proc toJson*(ctx: WorkerContext): JsonNode =
|
|||
"description": ctx.description
|
||||
}
|
||||
|
||||
const ContextDateFormat* = "yyyy-MM-dd'T'HH:mm:sszzz"
|
||||
|
||||
proc fromJson*(T: typedesc[WorkerContext], j: JsonNode): WorkerContext =
|
||||
## Deserialize context from JSON
|
||||
let dateStr = j["created_at"].getStr
|
||||
var createdAt: Time
|
||||
try:
|
||||
createdAt = parse(dateStr, ContextDateFormat).toTime()
|
||||
except TimeParseError:
|
||||
raise newException(TimeParseError,
|
||||
"Invalid date format '" & dateStr & "', expected " & ContextDateFormat)
|
||||
WorkerContext(
|
||||
taskId: j["task_id"].getStr,
|
||||
branch: j["branch"].getStr,
|
||||
worktree: j["worktree"].getStr,
|
||||
createdAt: parse(j["created_at"].getStr, "yyyy-MM-dd'T'HH:mm:sszzz").toTime(),
|
||||
createdAt: createdAt,
|
||||
description: j{"description"}.getStr
|
||||
)
|
||||
|
|
|
|||
|
|
@ -60,3 +60,20 @@ template withTransaction*(db: DbConn, body: untyped) =
|
|||
except CatchableError:
|
||||
db.exec("ROLLBACK")
|
||||
raise
|
||||
|
||||
# Error handling helpers
|
||||
proc logError*(operation, message: string) =
|
||||
## Log error to stderr with consistent format
|
||||
stderr.writeLine("ERROR: ", operation, ": ", message)
|
||||
|
||||
proc logWarn*(operation, message: string) =
|
||||
## Log warning to stderr with consistent format
|
||||
stderr.writeLine("WARN: ", operation, ": ", message)
|
||||
|
||||
template wrapError*(ExType: typedesc, operation: string, body: untyped) =
|
||||
## Wrap an operation, catching errors and re-raising with context.
|
||||
## Usage: wrapError(DbError, "openBusDb"): db = openDatabase(path)
|
||||
try:
|
||||
body
|
||||
except CatchableError as e:
|
||||
raise newException(ExType, operation & ": " & e.msg)
|
||||
|
|
|
|||
Loading…
Reference in a new issue