Skip to content

fix(oc-docs): contextual error messages for failed try-it requests (BRU-3408)#40

Open
sundram-bruno wants to merge 1 commit into
opencollection-dev:mainfrom
sundram-bruno:fix/bru-3408-try-it-error-messages
Open

fix(oc-docs): contextual error messages for failed try-it requests (BRU-3408)#40
sundram-bruno wants to merge 1 commit into
opencollection-dev:mainfrom
sundram-bruno:fix/bru-3408-try-it-error-messages

Conversation

@sundram-bruno

@sundram-bruno sundram-bruno commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

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 for AbortError, but AbortSignal.timeout() throws a TimeoutError, so real timeouts leaked the raw "signal timed out".

JIRA : https://usebruno.atlassian.net/browse/BRU-3408

Approach

Browser fetch collapses 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

Case Detected by Message
timeout abort/timeout signal "Request timed out. The server didn't respond in time."
mixed-content page https, target http "Request blocked: this page is secure (https) but the URL is insecure (http). Use an https URL, or run it from the Bruno desktop app."
browser-blocked cross-origin, or docs opened from a file "Request blocked by your browser, usually CORS: the API didn't allow requests from this page. Try it in the Bruno desktop app."
unreachable same-origin "Couldn't reach the server. It may be down, or the URL may be wrong."
unknown non-network error / unparseable URL the underlying error message

Same-vs-different is compared by origin (scheme+host+port), not site, because CORS is enforced per-origin (https://docs.example.comhttps://api.example.com is cross-origin even though both are example.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 dead network/ssl branches fetch can 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/5xx remain normal responses. Response display, default timeout, and OAuth2 handling are unchanged.

Tests

  • Unit (classifyRequestError.spec.ts): all five cases + edges (cross-origin, file origin, same-origin never-CORS, unparseable URL, non-Error throw).
  • E2E (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; uses page.route for deterministic failures.

All existing e2e (63) and unit tests pass; new code is lint-clean.

Acceptance criteria

  • Failure classified from request context (timeout; scheme; same-origin vs cross-origin; file)
  • Blanket "CORS error" removed; CORS only for cross-origin / file, never same-origin
  • timeout / mixed-content / browser-blocked / unreachable each show their message; anything else shows the underlying error
  • Only the failure message changes; response display, default timeout, OAuth2 unchanged
  • A test per classified case

…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 sundram-bruno force-pushed the fix/bru-3408-try-it-error-messages branch from f32a4ec to fbd659a Compare June 17, 2026 14:18
@sundram-bruno

Copy link
Copy Markdown
Contributor Author

Updated: rebased onto current main (picks up the centralized theming from #42) and squashed to a single commit.

The error banner is now styled entirely via theme.generated.css tokens — --oc-status-danger-text/--oc-status-danger-border for the red and --oc-border-border2 for the box border — so it matches Bruno desktop's script-error banner exactly (hsl(8,60%,52%) danger red, #cccccc border) and stays light/dark theme-aware. No hardcoded colors.

Verified live in collection docs: browser-blocked, unreachable, timeout, and unknown all render correctly; 13 unit + 3 e2e green.

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.

3 participants