Comfy-Org / Comfy-Org/ComfyUI_frontend

assert_mode is set on catch-based reports that are not assertions

Open
#17,289 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Problem / Goal

`assert_mode` is set on `reportError` calls that are catch blocks, not assertions.

ADR-TELEMETRY-ERRORS-0030 rule 2 scopes the tag to assertions: "`assert_mode`: `soft`, `hard`, or `sampled` for assertions". The actual assertion emitter, `reportAssertFailure` in `src/platform/telemetry/assertFailureReporter.ts`, sets `errorType: 'invariant_assert'` and no `assert_mode` at all.

The effect is that a query on `assert_mode:soft` returns only these catch-based reports and zero real assert failures — the opposite of what the tag name suggests. `failure_kind: 'degraded'` / `caught_unexpected` already carries the intent.

Known call sites on main:

- `src/platform/cloud/notification/components/DesktopCloudNotificationController.vue` — three reports, all catch blocks
- whatever #16997 landed with the same pattern

#17001 and #17000 removed the tag from their five call sites, so the family is currently inconsistent until these are cleaned up.

Note the ADR is Proposed and lives on branch `christian-byrne/tele-1-agent-telemetry-adr`, not main. If the lane instead decides `soft` should mean "any non-throwing report", the fix is to widen the ADR text and re-add the tag in #17001/#17000 — either direction is fine, but the two should not disagree.

## Proposed Solution

Pick one direction and apply it everywhere:

1. Follow the ADR — drop `assert_mode` from catch-based `reportError` calls, leaving it for `assert()`/`softAssert` paths once those emit it; or
2. Widen the ADR to say `soft` covers any non-throwing report, and re-add the tag to the five call sites that dropped it.

## Acceptance Criteria

- [ ] Every `reportError` call site in the repo agrees on whether catch blocks set `assert_mode`
- [ ] The ADR text and the code agree
- [ ] A query on `assert_mode` returns a set that matches the tag's documented meaning

---

Raised on #17001 (https://github.com/Comfy-Org/ComfyUI_frontend/pull/17001#discussion_r3941117060 and https://github.com/Comfy-Org/ComfyUI_frontend/pull/17001#discussion_r3942681233).

Contributor guide

Open the contributing guide

Research direction

Start by comparing the reportError call sites in DesktopCloudNotificationController.vue and the changes from #16997, #17001, and #17000 with reportAssertFailure in src/platform/telemetry/assertFailureReporter.ts. Read ADR-TELEMETRY-ERRORS-0030 on the christian-byrne/tele-1-agent-telemetry-adr branch, choose the consistent interpretation, and apply it across the repository. Done means the ADR, all reportError sites, and assert_mode query behavior agree.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.