Skip to content

Report which account a client error happened to - #1238

Merged
TaprootFreak merged 18 commits into
developfrom
feat/report-client-error-account
Aug 3, 2026
Merged

Report which account a client error happened to#1238
TaprootFreak merged 18 commits into
developfrom
feat/report-client-error-account

Conversation

@TaprootFreak

@TaprootFreak TaprootFreak commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #1237 (frontend half).

Depends on DFXswiss/api#4594 — that one must be merged and deployed first. The global validation
pipe strips a property the DTO does not declare, so sending this field before then would look like
it works and record nothing.

What

A client error report now carries the signed-in account, so a recorded failure can be tied to the
customer who calls support about it. Omitted entirely when nobody is signed in.

How

reportClientError takes the account as an argument and stays a plain function that reads no
React context: it also runs from the window listeners installed at startup, and a failure during
startup — before any session is loaded — is exactly what this reporting exists to catch. Making it
depend on user context being loaded would lose that case.

The error screen passes the account, because that screen belongs to the tree that failed. Keeping
the value in the reporting module instead would have made it shared state: a module is evaluated
once, while Main.widget.tsx and Main.lib.tsx render one App per embedded instance.

The account is part of the deduplication signature for the same reason. The same failure under two
accounts is two customers, and dropping the second would hide exactly the one this reporting exists
to find. That signature is now composed with JSON.stringify rather than joined by a separator a
message is free to contain — otherwise two different failures could produce the same signature and
the second would be dropped as a repeat.

The value is left out unless it has the shape of an account id: a safe integer greater than zero,
matching what the endpoint accepts. Losing a report over the field that only helps to find it would
defeat the point of sending it.

What this value is not

The ingest endpoint is unauthenticated on purpose, so this is a correlation hint that anyone could
post — never an ownership or authorization signal. The API side documents the same.

What it does not cover

A failure that happens before the session has loaded is reported without an account: the report
goes out as soon as the failure is known, and waiting for a session would risk losing it.

So is a chunk failure that reaches the window listeners rather than the router — including a failed
dynamic import from a click handler, where the customer may well be signed in. Covering that would
mean handing the reporter a session getter from the standalone entry point, evaluated at the time
of the event. It is deliberately left out here: the listeners are installed before any session
exists, and the review that raised it agreed the obvious alternative — keeping the account in the
reporting module — is the design this PR already rejected, because the embedded builds share that
module across instances. Worth doing as a follow-up if the gap matters in practice.

Tests

  • the error screen reports the account of the customer who hit the failure, and reports without one
    when nobody is signed in
  • the account is sent, and left out for null, zero, a negative number and one past the safe integer
    range
  • the same failure is reported again for a different account, and only once for the same one
  • two failures whose values contain the separator stay apart, and an empty error type stays apart
    from a missing one
  • a report still goes out when the thrown value carries a name, message or stack that is not text,
    a message that throws when it is read, or a toString that throws — the last of these used to
    cost the report entirely, which the old test did not notice because it only asserted that nothing
    escaped
  • a value that cannot even be classified still yields its message, so a chunk failure hidden behind
    an unreadable field keeps both its report and the reload that recovers the customer
  • toErrorFacts converts the fields it can and drops the ones it cannot, and reads a message off a
    thrown object that is not an Error — previously reported as [object Object]

npm run lint, npm run test (691 tests, 63 suites), npm run build:dev and npm run widget:dev
all pass locally.

@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Fourteen review passes were needed to reach zero findings. Most of them were spent on one theme: reading a value somebody else threw, without letting that reading cost the report.

The substantive findings, in the order they came up:

  • The first design kept the account in the reporting module. That module is loaded once per page, while the embedded builds mount one tree per instance — a report could have carried another customer's account. Since a wrong attribution is worse than a missing one on a feature whose only purpose is attribution, the account is now passed in, read by the error screen from the tree that actually failed.
  • The deduplication signature ignored the account, so the same failure under two accounts was reported once. It also joined its parts with a separator a message is free to contain, which could merge two different failures into one. It is composed as JSON now, with the account in it.
  • Composing that signature could throw on a value with a name that does not serialise — which would have swallowed the whole report. That widened into the real lesson here: classification touches properties before any field is read, so the entire reading is guarded now, and what survives the failure is still used. A chunk failure hidden behind an unreadable field keeps both its report and the reload that recovers the customer.
  • ?? where || belonged, in three places: an empty message suppressed the fallback that would have named the failure.
  • A thrown object carrying a message was reported as [object Object]; it now yields the message. That is a plain gain for debugging, unrelated to the account.
  • A number of comments asserted more than the code holds — about other repositories, about libraries, about guarantees the endpoint never makes. Shortened rather than reworded again.

Two decisions are deliberate and documented in the description: a failure reported before the session has loaded carries no account, and the window listeners report without one. Both were weighed against the alternative and lost to it.

@TaprootFreak
TaprootFreak marked this pull request as ready for review August 2, 2026 14:32
@TaprootFreak
TaprootFreak merged commit 39e6d3e into develop Aug 3, 2026
6 checks passed
@TaprootFreak
TaprootFreak deleted the feat/report-client-error-account branch August 3, 2026 15:58
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.

Client error reports carry no account id, so a reported failure cannot be tied to the customer who hit it

1 participant