Skip to content

feat(logs): add --errors, backed by one shared isError predicate - #40

Merged
linyiru merged 2 commits into
mainfrom
feat/logs-errors-filter
Jul 31, 2026
Merged

feat(logs): add --errors, backed by one shared isError predicate#40
linyiru merged 2 commits into
mainfrom
feat/logs-errors-filter

Conversation

@linyiru

@linyiru linyiru commented Jul 31, 2026

Copy link
Copy Markdown
Member

Answers question #3 of the nii-course-system Error 1101 report (2026-07-30).

Problem

creek metrics and creek logs answered the same question with different rules:

rule
tail-worker/src/analytics.ts:isError → AE double2SUM(...) AS errs outcome !== "ok" or exceptions.length > 0 or status >= 500
control-plane/.../query.ts:matchesQuery outcome only

The tenant saw 40 errors in creek metrics and got nothing from creek logs --outcome exception, because their failing entries looked like this:

{"outcome":"ok","request":{"url":".../dashboard","method":"GET"},
 "exceptions":[{"name":"Error","message":"Network connection lost."}]}

They had to dump everything and grep it themselves.

The gap had no name

This was not a broken filter — it was a missing one.

"Did this request go wrong" is a derived notion spanning outcome, exceptions and HTTP status. --outcome is modelled on Cloudflare's TailOutcome enum (VALID_OUTCOMES is literally those 8 values) and answers a narrower question. Nothing in the CLI could express the derived one — which is exactly what someone reaches for after seeing a number in creek metrics.

So --errors is added alongside --outcome, not as a widening of it.

What was deliberately not done

Rewriting outcome at write time so the two agree. outcome is Cloudflare's fact, passed through verbatim (tail-worker/src/index.ts:75). "Responded fine, then threw" is genuinely distinct from "the invocation failed" — the tenant's entries were the former, thrown after the response started streaming. Overwriting it would destroy a real signal, and would make --outcome ok and --outcome exception overlap.

Where the predicate lives

isError now lives in @solcreek/sdk, which control-plane and the CLI both already depend on. That collapses the two filters logs-filter.ts already warned about —

"This module is the mirror of control-plane/.../query.ts:matchesQuery — if those drift, --follow shows different entries than --since for the same flags."

— into one shared function, so --errors --follow and --errors --since cannot disagree.

tail-worker keeps a mirror. It has no dependencies and cannot import the SDK — the same reason LogEntry is already re-declared there with a documented "change one, change the other" note. Both copies now name each other explicitly, and both are driven through the same case table, so editing either fails its own suite.

⚠️ A true single source would need a new shared package. I'm flagging that rather than doing it — LogEntry itself already exists in three places (tail-worker, control-plane, sdk), so consolidating is a real piece of work with its own blast radius, not a drive-by.

creek metrics now points at the answer

  Errors:        40 (0.5% of invocations)
                 └─ creek logs --errors --since 24h

The bare number was a dead end. That was the tenant's actual experience.

Testing

Verified by mutation — three independent breakages, three distinct failure sets:

mutation fails
server-side errorsOnly not applied control-plane filter test
--follow client filter not applied CLI filter test
exceptions clause dropped from the predicate sdk + control-plane + cli — proof the sharing is real

That last row is the point: one edit, three suites. It does not fail the tail-worker mirror, which is precisely the limitation documented above.

Tests include the tenant's exact entry shape asserted three ways: invisible to --outcome exception, visible to --errors, and --errors combining with --outcome as an AND rather than quietly relaxing it.

pnpm format:check, oxlint clean; typecheck clean on sdk/cli/tail-worker, control-plane unchanged at its 1 pre-existing better-auth error. Full suite green except the pre-existing packages/cli/src/dev/worker-runner.test.ts failures that reproduce on a clean main.

Note

--errors ships in the CLI, so tenants need a CLI upgrade to use it — unlike the recent server-side fixes. The creek metrics hint lands in the same release.

`creek metrics` and `creek logs` answered the same question with
different rules. A tenant saw "40 errors" in metrics and got an empty
list from `creek logs --outcome exception`, because their failing
entries were `outcome: "ok"` with a `Network connection lost.`
exception. They had to dump everything and grep it themselves
(reported 2026-07-30).

The cause was not a broken filter — it was a MISSING one. Error count
is a DERIVED notion spanning outcome, exceptions and HTTP status;
`--outcome` is modelled on Cloudflare's TailOutcome enum and cannot
express it. Nothing in the CLI could.

Adds `--errors` alongside `--outcome` rather than widening it, and
backs both `creek metrics` and the new filter with one predicate.

Deliberately NOT done: rewriting `outcome` at write time so the two
agree. `outcome` is Cloudflare's fact, passed through verbatim, and
"responded fine then threw" is genuinely distinct from "the invocation
failed" — the tenant's entries were the former. Overwriting it would
destroy a real signal and make `--outcome ok` / `--outcome exception`
overlap.

`isError` now lives in @solcreek/sdk, which control-plane and the CLI
both already depend on. That collapses the two filters logs-filter.ts
already warned about — "if those drift, --follow shows different
entries than --since for the same flags" — into one function, so
`--errors --follow` and `--errors --since` cannot disagree.

tail-worker keeps a mirror: it has no dependencies and cannot import
the SDK, same reason LogEntry is re-declared there. Both copies now
name each other, and both are driven through the same case table so an
edit to either fails its own suite. A true single source would need a
new shared package; flagged rather than done.

`creek metrics` now prints the matching drill-down command under a
non-zero error count. The number alone was a dead end — that was the
tenant's actual experience.

Each wiring point is independently covered: dropping the server filter,
the --follow filter, or the exceptions clause of the predicate each
fail a distinct set of assertions.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new --errors log filter that answers the derived question “did this request go wrong?” (based on outcome, exceptions, and HTTP status) and makes metrics and logs consistent by sharing a single canonical isError predicate via @solcreek/sdk.

Changes:

  • Introduces shared isError() in @solcreek/sdk, used by both control-plane historical filtering and CLI live-tail filtering.
  • Adds --errors to creek logs and wires it through server query parsing + client-side --follow filtering.
  • Keeps tail-worker’s write-side mirror of the predicate (AE double2) and adds parity tests; updates creek metrics output to point users to creek logs --errors.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/tail-worker/src/analytics.ts Documents and enforces the write-side mirrored isError used to stamp AE error counts.
packages/tail-worker/src/analytics.test.ts Adds parity tests to keep tail-worker’s isError behavior aligned with the SDK contract.
packages/sdk/src/types/index.ts Adds errors?: boolean to LogQueryFilters for the new shared filter.
packages/sdk/src/logs/is-error.ts Adds canonical shared isError predicate and minimal input shape (ErrorClassifiable).
packages/sdk/src/logs/is-error.test.ts Defines the contract table for isError behavior (tenant-reported edge case included).
packages/sdk/src/index.ts Re-exports the new logs predicate from the SDK public entrypoint.
packages/sdk/src/client/index.ts Serializes filters.errors as errors=1 in CreekClient.getLogs() requests.
packages/control-plane/src/modules/logs/types.ts Adds errorsOnly to server-side query shape with rationale.
packages/control-plane/src/modules/logs/query.ts Parses errors=1 and applies shared isError() in matchesQuery().
packages/control-plane/src/modules/logs/query.test.ts Adds server-side tests for errors=1 and its interaction with outcome filtering.
packages/cli/src/commands/metrics.ts Prints a hint guiding users from metrics error counts to creek logs --errors.
packages/cli/src/commands/logs.ts Adds the --errors CLI flag and passes it into LogQueryFilters.
packages/cli/src/commands/logs-filter.ts Applies shared isError() for --errors in live tail filtering to match server behavior.
packages/cli/src/commands/logs-filter.test.ts Adds CLI live-tail tests ensuring --errors parity and filter description output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 225 to 234
async getLogs(projectSlug: string, filters?: LogQueryFilters): Promise<LogQueryResponse> {
const url = new URL(`/projects/${projectSlug}/logs`, "http://x"); // base discarded by request()
if (filters?.since) url.searchParams.set("since", filters.since);
if (filters?.until) url.searchParams.set("until", filters.until);
if (filters?.deployment) url.searchParams.set("deployment", filters.deployment);
if (filters?.branch) url.searchParams.set("branch", filters.branch);
if (filters?.search) url.searchParams.set("search", filters.search);
if (filters?.limit !== undefined) url.searchParams.set("limit", String(filters.limit));
if (filters?.errors) url.searchParams.set("errors", "1");
for (const o of filters?.outcomes ?? []) url.searchParams.append("outcome", o);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Adopted in 609ec3805e — you found a real gap in my mutation coverage. I'd exercised the predicate, the server filter and the --follow filter, but nothing asserted the client actually puts errors on the wire; a silent drop there would leave --errors --since unfiltered while --errors --follow worked, which is the same inconsistency this PR removes, one layer down. getLogs had no serialization coverage at all, so the new block also pins the omission case (unset/false must send no param — the server reads === "1" and would silently ignore anything else) and that errors rides alongside outcome rather than replacing it. Verified by mutation: deleting the client line fails two of the three.

Nothing asserted that the client actually puts `errors` on the wire.
The predicate, the server-side filter and the `--follow` client filter
were all covered, so a client that silently dropped the param would
have left `--errors --since` returning unfiltered results while
`--errors --follow` filtered correctly — the same metrics-vs-logs
inconsistency this feature removes, reintroduced one layer down.

getLogs had no serialization coverage at all, so this also pins the
omission case: `errors` unset or false must send no param, since the
server reads `=== "1"` and would silently ignore anything else.

Raised in Copilot review of this PR; it found a real gap in the
mutation coverage.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/cli/src/commands/logs.ts:68

  • --errors is described as a general-purpose creek logs filter, but when --server/$CREEKD_URL is used the command routes to creekdLogs() (line 117) which doesn’t apply any structured filters. This can mislead users into thinking --errors works in creekd mode when it will be ignored. Clarify the help text (or explicitly reject the flag in creekd mode).
    errors: {
      type: "boolean",
      description:
        "Only failed requests — matches the error count in `creek metrics`. Broader than --outcome exception: also catches an exception thrown after the response started, and 5xx responses.",
    },

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

@linyiru
linyiru merged commit d0548cc into main Jul 31, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants