fix(profiling): Specific API error copy for flamegraphs and thresholds#114971
Open
JoshuaKGoldberg wants to merge 9 commits into
Open
fix(profiling): Specific API error copy for flamegraphs and thresholds#114971JoshuaKGoldberg wants to merge 9 commits into
JoshuaKGoldberg wants to merge 9 commits into
Conversation
Aggregate flamegraphs previously passed a literal error string into the warning overlay, profile fetch failures surfaced raw RequestError text, and the transaction threshold button could pass non-string responses to toasts after rate limiting. Introduce getRequestErrorUserMessage to prefer API detail and map common HTTP statuses (including 429) to friendly copy, then use it across these profiling paths. Refs PRO-41 Co-Authored-By: Cursor Agent <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
github-actions
Bot
added
the
Scope: Frontend
Contributor
📊 Type Coverage Diff
🔍 1 new type safety issue introducedType assertions (
This is informational only and does not block the PR. |
…ttp-error-messages
When an aggregate flamegraph request errored, both FlamegraphWarnings (inside AggregateFlamegraph) and the parent RequestStateMessageContainer rendered absolutely-positioned error overlays. Previously the inner overlay rendered the literal placeholder string "error" while the outer rendered "There was an error loading the flamegraph.", which masked the overlap. Aligning both on getRequestErrorUserMessage made the duplicate text visible. Suppress the parent error overlay when the flamegraph visualization is active, since FlamegraphWarnings already covers that case. The call tree visualization still falls back to the parent overlay. Refs PRO-41 Co-Authored-By: Cursor Agent <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
JoshuaKGoldberg
changed the title
… overlays When the API returned a `detail` for a failed aggregate flamegraph request, the overlay rendered only that detail (e.g. "Some specific detail."), with no surrounding indication that it was an error. Add an "Error loading flamegraph" heading above the message in both the inner FlamegraphWarnings overlay and the parent RequestStateMessageContainer (call tree view), so the framing matches the previous behavior while still surfacing the server-provided detail. Refs PRO-41 Co-Authored-By: Cursor Agent <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…erlays Replace the previously-introduced h4 heading in the flamegraph error overlays with a `role="alert"` live region. The h4 skipped levels from the page h1, and an error state shown conditionally on request failure is better represented as an alert than as part of the document heading outline. Title text is now rendered with Text bold (no heading semantics), while the alert role ensures the message is announced to screen readers when it appears. Applies to FlamegraphWarnings (inner overlay shared by aggregate, continuous, and single-profile flamegraphs) and the parent overlays used by the call tree view in the profiling landing page and the transaction summary profiling tab. Refs PRO-41 Co-Authored-By: Cursor Agent <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Increase the vertical gap between the alert title and detail message in the call-tree parent overlays from xs to md to give the two lines more visual separation. Refs PRO-41 Co-Authored-By: Cursor Agent <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Match the md vertical gap used in the call-tree parent overlay so the alert title and detail message (and other multi-row warning states such as the empty-data export button and no-frames filter reset) are spaced consistently inside the flamegraph warning overlay grid. Refs PRO-41 Co-Authored-By: Cursor Agent <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The single-profile flamegraph "renders a missing profile" test was
asserting on the raw String(err) output ("RequestError: GET …") that
TransactionProfileProvider emitted before profilesProvider switched to
getRequestErrorUserMessage. Assert on the alert title and the 404
mapping copy instead, so the test exercises the same user-facing error
path the UI now renders.
Refs PRO-41
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Profiling surfaces that load flamegraphs or transaction thresholds didn't show specific/tailored text for API errors:
RequestErrorstringificationThis change unifies a bunch of the error copy rendering to prefer, in order:
detailproperty is present, use that429), render a known message for that429500With server detail
500Without server detail
Fixes PRO-41.
Made with Cursor