fix(oc-docs): contextual error messages for failed try-it requests (BRU-3408)#40
Merged
bijin-bruno merged 1 commit intoJun 22, 2026
Conversation
sachin-bruno
approved these changes
Jun 11, 2026
…RU-3408) Browser fetch collapses CORS, DNS, connection-refused, offline and TLS into one opaque "Failed to fetch", so the old code mislabeled every failure as a definite "CORS error" and never caught timeouts. Classify failures from request context instead of the opaque message: - timeout, mixed-content, browser-blocked (CORS / file origin), unreachable (same-origin), uncategorized - never assert CORS as certain; same-origin failures never mention CORS - render a danger banner inside the Response tab (tab shell preserved, status bar hidden), matching Bruno desktop's error banner - style via theme.generated.css tokens (--oc-status-danger-*, --oc-border-border2) Adds classifyRequestError unit tests and Playwright e2e for the browser-blocked, unreachable and 4xx-is-not-a-failure cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sundram-bruno
force-pushed
the
fix/bru-3408-try-it-error-messages
branch
from
f32a4ec to
fbd659a
Compare
Contributor
Author
|
Updated: rebased onto current The error banner is now styled entirely via Verified live in collection docs: browser-blocked, unreachable, timeout, and unknown all render correctly; 13 unit + 3 e2e green. |
bijin-bruno
approved these changes
Jun 17, 2026
bijin-bruno
approved these changes
Jun 22, 2026
Contributor
|
E2E folder structure need to be updated. To be done in PR #44. @sundram-bruno |
Contributor
Author
will take care of it. |
sundram-bruno
added a commit
to sundram-bruno/opencollection
that referenced
this pull request
Jun 24, 2026
…e (BRU-3408)
The try-it failure-message spec was the last flat e2e file. Move it onto the
page-object structure and locate by test id / role instead of CSS classes:
- New e2e/components/request-playground.component.ts (RequestPlaygroundComponent)
— open try-it for an endpoint, send, edit URL, read the failure banner /
Response tab — exposed as the `requestPlayground` fixture.
- Move e2e/request-errors.spec.ts -> e2e/tests/request/request-errors.spec.ts;
it imports { test, expect } from the playwright harness and drives the
request playground. Assertions unchanged.
- Add data-testid to the error banner (error-banner / error-title /
error-message / error-hint) and the endpoint section so the spec no longer
relies on .error-title / .error-message / .endpoint-section class selectors.
BRU-3408 shipped in the merged PR opencollection-dev#40; this test-structure cleanup rides along
with the topbar PR (opencollection-dev#44) per request. Gates green: 88 unit, 17 e2e,
build:standalone.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bijin-bruno
pushed a commit
that referenced
this pull request
Jun 26, 2026
* feat(oc-docs): add sticky top navigation bar (BRU-3572)
Add a reusable, purely-presentational Topbar mounted into the
OpenCollection desktop shell. Composed of small subcomponents:
Brand (logo + collection name + version), an Open-in-Bruno CTA
(restyled from FetchInBrunoButton, renders a real bruno:// deep
link), and two slots — searchSlot (BRU-3573) and envSwitcherSlot
(BRU-3186) — that render whatever node is passed and degrade
gracefully when empty.
Cross-lane contract (src/components/Topbar/Topbar.tsx) kept stable
for BRU-3188: collectionName, version, logo, searchSlot,
envSwitcherSlot, onOpenInBruno, onToggleSidebar — plus an optional
openInBrunoHref so the CTA can render as an anchor with a real
bruno:// href (backward-compatible additive prop).
Responsive (no mobile/tablet Figma — designed here):
- desktop (>=1024): full bar, centered search, inline env + CTA.
- tablet (768-1023): sidebar inline (no hamburger), CTA icon-only,
env switcher stays inline.
- mobile (<768): hamburger + brand + spacer + search icon + overflow
+ CTA icon. Search icon expands a full-width search row; the
overflow popover relocates the same envSwitcherSlot node.
Styling maps every value to --oc-* theme tokens (no hardcoded hex):
square bar with bottom border only, 6px rounded inner controls,
Inter, honors light/dark. Remove the now-duplicate in-content
"Fetch in Bruno" button from Docs (Topbar owns the CTA).
Tests: vitest for buildBrunoDeepLink + layoutModeForWidth; Playwright
for sticky behavior, the bruno:// CTA href, mobile condense/overflow,
and search-expand (via a dev-only topbar harness page).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(oc-docs): pin Open-in-Bruno CTA to the right with empty slots (BRU-3572)
With searchSlot/envSwitcherSlot undefined there was no flex-1 element
between the brand and the right cluster, so the CTA collapsed next to
the brand on the left. Always render a flex-1 middle (inline search when
present, else a spacer) so the right-hand controls + CTA stay pinned to
the right edge regardless of slot contents. Add a desktop e2e asserting
the CTA hugs the right edge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(oc-docs): initials brand avatar with Bruno amber gradient (BRU-3572)
Default brand mark is now an initials badge derived from the collection
name (Brand renders it when no explicit logo is passed; a logo prop still
overrides). Matches the Figma brand mark: 26x26, 6px radius, white
initials over the Bruno amber linear gradient.
Initials: first letter of the first two words ("Hotel Booking API" -> HB,
"Bruno Testbench" -> BT); a single word uses just its first letter
("Echo" -> E). Pure getInitials() with unit tests.
Gradient stops map to existing theme tokens — no hardcoded hex:
--oc-brand (#D37F17 light) -> --oc-primary-subtle (#DC9741 light),
text via --oc-button2-color-primary-text (white light / black dark), so
the badge is fully theme-aware (verified light + dark).
Drop the hardcoded opencollection logo from the dev harness so the live
app shows the initials avatar by default. Add an e2e asserting the BT
avatar renders.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(oc-docs): display collection version with a leading "v" (BRU-3572)
Brand now formats the version for display as "v1.0.0" instead of raw
"1.0.0", matching Figma. Idempotent — a version already prefixed
("v2") is left unchanged. The Topbar `version` prop stays the raw
string; only the rendered label is formatted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(oc-docs): address self-review on topbar (BRU-3572)
Fixes from code review:
Bugs:
- Render the Open-in-Bruno CTA only when a deep link or onClick is
provided, instead of a visible-but-inert button when neither is set.
- Reset the mobile search row when leaving the mobile layout so it
doesn't reappear with stale aria state on the next mobile resize.
- MobileOverflow outside-dismiss now listens on pointerdown (covers
touch) instead of mousedown.
Optimization:
- useTopbarLayout only setState on an actual breakpoint-band crossing
(updater bail-out), and drop the redundant on-mount resize call.
Dead code / clarity:
- Delete the now-orphaned FetchInBrunoButton (superseded by
OpenInBrunoButton).
- Correct the StyledWrapper comment: layout switching is in JSX, not
CSS; data-mode is for debugging/e2e. Token claim scoped to colors.
- Use --oc-font-size-xs for the initials avatar instead of 11px.
Reusability:
- Extract a shared IconButton (hamburger, search toggle, overflow).
- Deduplicate the search-inner wrapper in Topbar.
- Collapse OpenInBrunoButton's anchor/button branches into one render.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(oc-docs): Docs brand label, desktop-only CTA, hamburger below desktop (BRU-3572)
Per updated mobile/tablet design:
- Header brand now shows the fixed "Docs" label + initials avatar (HB),
not the collection name. Version is no longer rendered in the header
(it remains in the page body). collectionName still drives the avatar
initials and the accessible title.
- Open-in-Bruno is desktop-only — the Bruno desktop app isn't available
on tablet/mobile, so the CTA is hidden below the desktop breakpoint.
- Hamburger now shows on tablet too (sidebar is a drawer below desktop,
persistent only on desktop).
- Search collapses to an icon below desktop (tablet + mobile), expanding
a full-width row; inline search input is desktop-only. Env switcher
stays inline on tablet, in the overflow popover on mobile.
`version` is kept on TopbarProps for cross-lane contract stability but is
intentionally not rendered. Tests + harness updated for the new model.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* style(oc-docs): borderless ghost icon-buttons in topbar (BRU-3572)
Drop the 1px border on the shared topbar icon-button (hamburger, search
toggle, overflow trigger) to match the design — ghost style with a
hover/active background for affordance instead of a border.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(oc-docs): gate Open-in-Bruno on device capability, not width (BRU-3572)
iPad Pro is 1024–1366px wide, so the width-only desktop breakpoint showed
the Open-in-Bruno CTA on it. Split "layout mode" (width) from "can run the
Bruno desktop app" (capability): the CTA now needs the desktop layout AND a
capable device, so the iPad Pro gets the desktop layout with no CTA.
Capability (computeCanRunBrunoApp, pure + unit-tested):
- Require any fine, hovering pointer — `(any-hover: hover) and
(any-pointer: fine)` — so touchscreen laptops / 2-in-1s (touch is the
primary pointer but a trackpad exists) still qualify.
- Hard-exclude mobile/tablet OSes: UA match, plus maxTouchPoints > 1 on
MacIntel to unmask iPadOS 13+ (which reports as Mac). A real Mac
(maxTouchPoints === 0) is kept. The exclusion — not the media query — is
what reliably catches an iPad with a trackpad folio.
Also revert the header brand to collection name + version (per design); the
"Docs"/no-version variant is dropped. Layout breakpoints unchanged: large
tablets (>=1024) keep the desktop layout, 768–1023 keep hamburger + search
icon. Add a dev-only topbar-device-check.html that prints the raw signals +
the computed decision for verifying the device matrix (not shipped).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(oc-docs): align Topbar to per-component folder convention (BRU-3572)
Structure + naming only — no behavior, props, or visual change. Adopts the
house convention from PR #45 (each component a folder with <Name>.tsx +
<Name>.spec.tsx + StyledWrapper.ts + index.ts; pure utils in src/utils;
hooks in src/hooks; shared glyphs in src/assets/icons.tsx).
Moves:
- Promote shared primitives to top-level src/components/: IconButton/,
InitialsAvatar/, OpenInBrunoButton/ (reusable across lanes).
- Keep Topbar/, Topbar/Brand/, Topbar/MobileOverflow/ as their own folders.
- Split the single Topbar/StyledWrapper.ts into per-component StyledWrapper.ts.
- Pure utils -> src/utils: buildBrunoDeepLink, getInitials (+ .spec.ts).
- Hooks -> src/hooks: useTopbarLayout, useCanRunBrunoApp (+ .spec.ts).
- Raw glyphs -> src/assets/icons.tsx (SearchIcon, HamburgerIcon, OverflowIcon,
BrunoGlyph). Matches PR #45's icons.tsx style/baseIconProps; see the file's
reconciliation note for the (mechanical) merge with #45's glyph set.
- *.test.ts -> *.spec.ts(x); add a colocated render spec per component.
IconButton consolidation:
- Single canonical src/components/IconButton/ (icon-only button primitive,
aria-label required). Removed the duplicate, unused IconButton from
ui/MinimalComponents.tsx (the other exports there are untouched) and the
local copy in Topbar/icons.tsx. All callers import the shared path.
Dev scaffolding removed from shipped src/ + package root: deleted
src/topbarHarness.tsx, src/deviceCheck.tsx, topbar-harness.html,
topbar-device-check.html. The e2e harness is now gated behind dev.tsx
(?view=topbar-harness); e2e navigates there instead of the html.
Public API unchanged: Topbar still exports from src/components/Topbar
(index.ts re-export) for BRU-3188. OpenCollection updated to import
buildBrunoDeepLink from src/utils.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(oc-docs): compact mobile brand — avatar + "Docs" only (BRU-3572)
On mobile the brand showed the full collection name + version like
tablet/desktop. Per design, mobile shows just the initials avatar (HB)
and a fixed "Docs" label — no collection name, no version (both remain
in the page body). Add a `compact` prop to Brand and drive it from
`isMobile` in Topbar. Tablet/desktop keep name + version.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(oc-docs): address self-review on topbar capability + shadow (BRU-3572)
- useCanRunBrunoApp: re-detect pointer capability via the media query's
own `change` event instead of window `resize` (a resize event does not
fire when a trackpad / external mouse is attached, so the documented
dynamic re-detection never ran). Initial-load detection is unchanged.
- MobileOverflow popover: use the --oc-shadow-md theme token instead of a
hardcoded rgba shadow, so the elevation adapts in dark mode.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(oc-docs): align topbar header to design — hamburger, spacing, avatar (BRU-3572)
Matched the Claude design reference (chrome.jsx):
- IconButton glyph now uses the base text token (--oc-text, = design's
#343434 fg-base in light) instead of the muted token, and renders at
18px — the hamburger/search/overflow glyphs read solid, not faded.
- Mobile bar tightened to match: padding 0 12px + gap 8 (from 20/12), and
the hamburger gets margin-left -4 so the brand isn't over-indented.
- Brand logo↔text gap 10 -> 8.
- Initials avatar: gradient is now a fixed brand mark (#d37f17 -> #dc9741,
identical in light/dark like the Bruno mascot), white text, JetBrains
Mono with -0.02em tracking — matches the design letterforms (was Inter,
theme-tokened, +0.02em).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(oc-docs): use data-testid selectors in topbar e2e (BRU-3572)
Per team convention, drop tag/class-based Playwright selectors
(page.locator('header.oc-topbar'), '.oc-topbar__brand-name', etc.) in
favor of data-testid. Adds data-testid to the Topbar root ("topbar"),
Brand ("brand"), brand name ("brand-name") and version ("brand-version")
and switches the e2e to getByTestId. Also replaces the emotion-class
assertion on the CTA (toHaveClass(/is-full/)) with a visibility check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(oc-docs): drop per-component index.ts barrels per review (BRU-3572)
Team standard (reversed from the earlier 4-file pattern): component folders
have no index.ts barrel; consumers import the concrete file directly.
- Delete the 6 barrels this PR added: Topbar/, Topbar/Brand/,
Topbar/MobileOverflow/, IconButton/, InitialsAvatar/, OpenInBrunoButton/.
- Repoint every importer at the concrete module (e.g. '../Topbar' ->
'../Topbar/Topbar', '../IconButton' -> '../IconButton/IconButton'),
switching the barrel's named re-exports back to the components' default
exports. Touches Topbar.tsx, Brand.tsx, MobileOverflow.tsx,
OpenCollection.tsx (mount) and dev.tsx (harness).
Behavior/visual unchanged. icons split + e2e locators restructure deferred
to mirror PR #45 once it lands (icons.tsx already notes the reconciliation).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(oc-docs): scope header e2e to shipped behavior + adopt e2e structure (BRU-3572)
Restructure the header Playwright suite onto the project's e2e folder
layout. Add the harness infra (as per the established e2e structure):
e2e/config/app.config.ts, e2e/components/base.component.ts,
e2e/playwright/{index,pages.fixture}.ts, e2e/tsconfig.json; point
playwright.config.ts at appConfig.
- New e2e/components/page-header.component.ts — a PageHeader layout
component exposed as the `pageHeader` fixture, so specs read
`pageHeader.brandName` directly (the model a future `sidebar` fixture
will follow). It owns only the header's shipped chrome: brand cluster,
Open-in-Bruno CTA, and the menu (hamburger) trigger.
- Move e2e/topbar.spec.ts -> e2e/tests/layout/page-header.spec.ts.
Scope the suite to what the mounted app actually renders for this ticket:
brand name/version, initials avatar, Open-in-Bruno (bruno:// href,
desktop-only, pinned right), and mobile condense (hamburger shows, CTA
hidden, compact "Docs" brand). Dropped the slot-fill tests (search expand,
overflow env, hamburger callback): those need the search input (BRU-3573),
env switcher (BRU-3186) and sidebar drawer (BRU-3574), so they belong with
those tickets — and removed the now-orphaned dev harness from dev.tsx.
All gates green: 4 header e2e + full suite (69) + build:standalone.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(oc-docs): eager capability check + note slot/toggle gaps (BRU-3572)
Pre-push review fixes:
- useCanRunBrunoApp measures the device capability eagerly on first render
(SSR/no-window safe), so the Open-in-Bruno CTA's visibility is correct on
first paint — removes a one-tick flash and makes the desktop e2e
deterministic rather than retry-dependent.
- Comment at the Topbar mount noting searchSlot (BRU-3573), envSwitcherSlot
(BRU-3186) and onToggleSidebar (BRU-3574) are wired by their own tickets.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(oc-docs): migrate request-errors e2e to the page-object structure (BRU-3408)
The try-it failure-message spec was the last flat e2e file. Move it onto the
page-object structure and locate by test id / role instead of CSS classes:
- New e2e/components/request-playground.component.ts (RequestPlaygroundComponent)
— open try-it for an endpoint, send, edit URL, read the failure banner /
Response tab — exposed as the `requestPlayground` fixture.
- Move e2e/request-errors.spec.ts -> e2e/tests/request/request-errors.spec.ts;
it imports { test, expect } from the playwright harness and drives the
request playground. Assertions unchanged.
- Add data-testid to the error banner (error-banner / error-title /
error-message / error-hint) and the endpoint section so the spec no longer
relies on .error-title / .error-message / .endpoint-section class selectors.
BRU-3408 shipped in the merged PR #40; this test-structure cleanup rides along
with the topbar PR (#44) per request. Gates green: 88 unit, 17 e2e,
build:standalone.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(oc-docs): align topbar to the team folder conventions (BRU-3572)
Adopt the structure conventions from the EM review (Bijin):
- Icons: split src/assets/icons.tsx into one file per icon under
src/assets/icons/ (SearchIcon/HamburgerIcon/OverflowIcon/BrunoGlyph) +
shared baseIconProps.ts + an index.ts aggregator (importable individually
or from the barrel). Import paths unchanged.
- ui/ vs components/: IconButton is a generic presentational primitive →
moved to src/ui/IconButton/. Feature components (Topbar/Brand/
MobileOverflow) stay under src/components/Topbar/.
- StyledWrapper naming: every per-component styled root export is now named
StyledWrapper (was StyledButton / Base / Badge in IconButton /
OpenInBrunoButton / InitialsAvatar).
- Default testId prop: Topbar, Brand, InitialsAvatar and OpenInBrunoButton
take an optional `testId` (defaults equal the previous hardcoded ids, so
existing selectors keep matching); Brand derives `${testId}-name` /
`${testId}-version`.
- E2E components grouped by area: e2e/components/{layout,request}/*.
Structure/naming only — no visual or behavior change. Gates green:
88 unit, 17 e2e, build:standalone.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Rewrite pages.fixture comment to document present fixtures
Drop the comparison against `layout.header.brandName` (an API that never
existed) so the comment describes what the code actually does.
* Assert the header bounding box exists before checking its position
`box?.y ?? -1` passed the sticky check even when boundingBox() returned
null (element missing). Assert non-null first, then check y.
* Guard the header bounding box with an explicit null check
Replace the non-null assertion with an early throw so a missing
boundingBox() fails loudly instead of being silently asserted.
* Move getInitials into utils/common.ts
Fold the helper and its tests into the shared common module instead of a
single-function file; update the InitialsAvatar import.
* Drop the oc- prefix from topbar class names
Rename the .oc-topbar__* selectors and classNames to short names
(topbar-bar, topbar-spacer, topbar-brand-name, ...) across the topbar
stylesheets and components. Theme tokens (--oc-*) are unchanged.
* Move Topbar into src/ui and strip stylesheet comments
- Relocate the Topbar component (Topbar, Brand, MobileOverflow) from
src/components to src/ui; fix the affected relative imports.
- Remove all comments from the topbar/ui StyledWrapper stylesheets.
* Select the sidebar toggle by test id
Add data-testid="topbar-menu" to the hamburger button and locate it via
getByTestId instead of the accessible-name role query.
* Add a 2px gap between the brand name and version
Separate the stacked collection name and version in the topbar brand.
* Move Topbar back to components/
ui/ is for generic, presentational primitives with minimal deps (Button,
Dropdown-style). Topbar depends on project hooks, assets and feature
components, so it belongs under components/. IconButton stays in ui/.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.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
3 participants
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.
Problem
A failed try-it request in the collection docs showed only an opaque "Request Failed / Failed to fetch", and the runner hardcoded every network failure as a
"CORS error"— including connection-refused, same-origin, and timeouts (see opencollection #24). Timeouts were also missed: the catch checked forAbortError, butAbortSignal.timeout()throws aTimeoutError, so real timeouts leaked the raw"signal timed out".JIRA : https://usebruno.atlassian.net/browse/BRU-3408
Approach
Browser
fetchcollapses CORS, DNS, connection-refused, offline, and TLS into one opaque failure with no detail — the real cause lives only in devtools. So we classify from the request context the browser does expose (timeout, page vs target scheme, same-origin vs cross-origin / file), not the error text.Five cases
https, targethttpSame-vs-different is compared by origin (scheme+host+port), not site, because CORS is enforced per-origin (
https://docs.example.com→https://api.example.comis cross-origin even though both areexample.com). CORS is suggested only for cross-origin or opened-from-file failures, never same-origin (AC #2).Changes
classifyRequestError— pure classifier returning{ type, title, message }; takes the resolved request URL + the page URL (window.location.href, passed in to stay pure). Unparseable URL → underlying message.RequestExecutor— catch delegates to the classifier; removed the hardcoded CORS string and the deadnetwork/sslbranchesfetchcan never produce.ErrorBanner— small reusable UI (bold title + monospace message) mirroring Bruno desktop's response error banner.ResponsePane— banner renders inside the Response tab (consistent with a success response); status bar hidden on failure (no HTTP status exists).4xx/5xxremain normal responses. Response display, default timeout, and OAuth2 handling are unchanged.Tests
classifyRequestError.spec.ts): all five cases + edges (cross-origin, file origin, same-origin never-CORS, unparseable URL, non-Error throw).request-errors.spec.ts): cross-origin → browser-blocked (inside Response tab), same-origin → unreachable (never mentions CORS), 4xx → renders normally. First e2e to actually drive Try → Send; usespage.routefor deterministic failures.All existing e2e (63) and unit tests pass; new code is lint-clean.
Acceptance criteria