fix(chdb): return error on log_scores row scan failure
ci/woodpecker/push/woodpecker Pipeline was successful
ci/woodpecker/push/woodpecker Pipeline was successful
Previously a per-row scan error was logged and skipped with continue, so a decode failure (e.g. the UInt64->int64 regression) produced a 200 response with partial or empty data and no signal to the client. Fail the request instead so the HTTP handlers return 500. Also check rows.Err() after iteration.
This commit is contained in:
+20
-6
@@ -122,13 +122,19 @@ func (d *ClickHouse) Logscores(ctx context.Context, serverID, monitorID int, sin
|
|||||||
for rows.Next() {
|
for rows.Next() {
|
||||||
row, err := scanLogScore(rows)
|
row, err := scanLogScore(rows)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Error("could not parse row", "err", err)
|
// Fail the whole request so the caller returns a 500 rather than
|
||||||
continue
|
// silently serving partial/empty data. If this turns into frequent
|
||||||
|
// user-facing errors, we could tolerate a few (or a small
|
||||||
|
// percentage of) row errors per request when the rest is good data.
|
||||||
|
log.ErrorContext(ctx, "could not parse row", "err", err)
|
||||||
|
return nil, fmt.Errorf("could not parse log_scores row: %w", err)
|
||||||
}
|
}
|
||||||
rv = append(rv, row)
|
rv = append(rv, row)
|
||||||
}
|
}
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
// log.InfoContext(ctx, "returning data", "rv", rv)
|
log.ErrorContext(ctx, "rows error", "err", err)
|
||||||
|
return nil, fmt.Errorf("database error")
|
||||||
|
}
|
||||||
|
|
||||||
return rv, nil
|
return rv, nil
|
||||||
}
|
}
|
||||||
@@ -210,11 +216,19 @@ func (d *ClickHouse) LogscoresTimeRange(ctx context.Context, serverID, monitorID
|
|||||||
for rows.Next() {
|
for rows.Next() {
|
||||||
row, err := scanLogScore(rows)
|
row, err := scanLogScore(rows)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Error("could not parse row", "err", err)
|
// Fail the whole request so the caller returns a 500 rather than
|
||||||
continue
|
// silently serving partial/empty data. If this turns into frequent
|
||||||
|
// user-facing errors, we could tolerate a few (or a small
|
||||||
|
// percentage of) row errors per request when the rest is good data.
|
||||||
|
log.ErrorContext(ctx, "could not parse row", "err", err)
|
||||||
|
return nil, fmt.Errorf("could not parse log_scores row: %w", err)
|
||||||
}
|
}
|
||||||
rv = append(rv, row)
|
rv = append(rv, row)
|
||||||
}
|
}
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
log.ErrorContext(ctx, "rows error", "err", err)
|
||||||
|
return nil, fmt.Errorf("database error")
|
||||||
|
}
|
||||||
|
|
||||||
log.InfoContext(
|
log.InfoContext(
|
||||||
ctx, "time range query results",
|
ctx, "time range query results",
|
||||||
|
|||||||
Reference in New Issue
Block a user