Skip to content

[superlog] Log batch_union fallback as WARN instead of ERROR #513

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.

Sign up for GitHub

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

Merged
izadoesdev merged 1 commit into from
Jun 30, 2026
Merged

[superlog] Log batch_union fallback as WARN instead of ERROR #513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/ai/src/lib/tracing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,29 @@ export function mergeWideEvent<Fields extends object = Record<string, unknown>>(
log.info({ service: "api", ...payload });
}

export function captureWarning<Fields extends object = Record<string, unknown>>(
error: unknown,
fields?: Partial<Fields>
): void {
const err = error instanceof Error ? error : new Error(String(error));
const payload = fields as Record<string, unknown> | undefined;
const requestLog = getActiveAiRequestLogger();
if (requestLog) {
if (payload) {
requestLog.warn(err.message, payload);
} else {
requestLog.warn(err.message);
}
return;
}
log.warn({
service: "api",
error_message: err.message,
...(err.stack ? { error_stack: err.stack } : {}),
...(payload ?? {}),
});
Comment on lines +31 to +36

@greptile-apps greptile-apps Bot Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 captureWarning's fallback path includes error_stack in the structured log object, but the sibling captureError function's fallback log.error call does not. This means a recovered warning ends up with more stack context in the fallback logger than a genuine error, which inverts expectations and could make error triage harder when the request logger is absent.

Suggested change
log.warn({
service: "api",
error_message: err.message,
...(err.stack ? { error_stack: err.stack } : {}),
...(payload ?? {}),
});
log.warn({
service: "api",
error_message: err.message,
...(payload ?? {}),
});

}

export function captureError<Fields extends object = Record<string, unknown>>(
error: unknown,
fields?: Partial<Fields>
Expand Down
44 changes: 44 additions & 0 deletions packages/ai/src/query/batch-executor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { chQuery } from "@databuddy/db/clickhouse";
import type { RequestLogger } from "evlog";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { setAiRequestLoggerProvider } from "../lib/request-logger";
import {
areQueriesCompatible,
buildUnionQuery,
Expand Down Expand Up @@ -55,6 +57,7 @@ function transientClickHouseError(): Error {

beforeEach(() => {
mockChQuery.mockReset();
setAiRequestLoggerProvider(null);
});

describe("batch-executor schema signatures", () => {
Expand Down Expand Up @@ -182,6 +185,47 @@ describe("executeBatch single query retry", () => {
});
});

describe("executeBatch union fallback logging", () => {
it("logs batch union fallback as a warning when single queries recover", async () => {
const mockLogger = {
error: vi.fn(),
set: vi.fn(),
warn: vi.fn(),
} as unknown as RequestLogger;
setAiRequestLoggerProvider(() => mockLogger);
mockChQuery
.mockRejectedValueOnce(new Error("Union query failed"))
.mockResolvedValueOnce([{ name: "/", pageviews: 2, visitors: 1 }])
.mockResolvedValueOnce([{ name: "/pricing", pageviews: 1, visitors: 1 }]);

const results = await executeBatch([
{ ...singleQueryRequest, type: "top_pages" },
{ ...singleQueryRequest, type: "top_pages" },
]);

expect(chQuery).toHaveBeenCalledTimes(3);
expect(mockLogger.warn).toHaveBeenCalledWith(
"Union query failed",
expect.objectContaining({
batch_size: 2,
batch_types: "top_pages,top_pages",
operation: "batch_union",
})
);
expect(mockLogger.error).not.toHaveBeenCalled();
expect(results).toEqual([
{
type: "top_pages",
data: [{ name: "/", pageviews: 2, visitors: 1 }],
},
{
type: "top_pages",
data: [{ name: "/pricing", pageviews: 1, visitors: 1 }],
},
]);
});
});

describe("buildUnionQuery compile isolation", () => {
const baseRequest = {
projectId: "test-website",
Expand Down
11 changes: 7 additions & 4 deletions packages/ai/src/query/batch-executor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { chQuery } from "@databuddy/db/clickhouse";
import { captureError, mergeWideEvent } from "../lib/tracing";
import { captureWarning, mergeWideEvent } from "../lib/tracing";
import { QueryBuilders, suggestQueryTypes } from "./builders";
import {
getClickHouseQuerySettings,
Expand Down Expand Up @@ -529,15 +529,18 @@ export async function executeBatch(
}
return { unionCount: 1, singleCount: 0 };
} catch (error) {
captureError(error, {
const batchUnionError =
error instanceof Error ? error.message : "Union query failed";
captureWarning(error, {
operation: "batch_union",
batch_types: compiledItems.map((g) => g.req.type).join(","),
batch_size: compiledItems.length,
batch_union_fallback: 1,
batch_union_error: batchUnionError,
});
mergeWideEvent({
batch_union_fallback: 1,
batch_union_error:
error instanceof Error ? error.message : "Union query failed",
batch_union_error: batchUnionError,
});
for (const { index, req } of compiledItems) {
results[index] = await runSingle(req, opts);
Expand Down
Loading