From cc58854128b20a45fd2c3c0d5392a6f61b9a0d2c Mon Sep 17 00:00:00 2001 From: Sundram Gupta Date: Wed, 10 Jun 2026 01:53:21 +0530 Subject: [PATCH 1/2] fix(oc-docs): contextual error messages for failed try-it requests (BRU-3408) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed try-it request previously showed an opaque "Request Failed / Failed to fetch" and hardcoded every network failure as a "CORS error" — including connection-refused and timeouts. Timeouts were also missed entirely (the catch checked for `AbortError`, but `AbortSignal.timeout()` throws a `TimeoutError`), so they leaked the raw "signal timed out". Browser fetch collapses CORS, DNS, connection-refused, offline, and TLS into one opaque `TypeError: Failed to fetch`, so only two states are actually distinguishable in code: timeout and opaque. Anything else is reported as uncategorized rather than guessed at. Changes: - Add `classifyRequestError` — a pure classifier returning a structured { type, title, message, hint } for the three states (timeout / opaque / uncategorized). The opaque message is CORS-first but hedged, and names the file-origin `null` case, without asserting CORS as certain. - RequestExecutor delegates its catch to the classifier; remove the hardcoded CORS string and the dead network/ssl branches that fetch can never produce. - Add a reusable `ErrorBanner` UI (bold title, monospace message, optional next-step hint) mirroring Bruno desktop's response error banner. - Render the banner inside the Response tab (keeping the tab shell) and hide the status bar on failure, since no HTTP status exists. - 4xx/5xx remain normal responses (not failures). Tests: - Unit: classifyRequestError — all three states plus edge cases. - E2E: opaque banner renders in the Response tab, never mislabels as a definite CORS error, and a 4xx renders normally. Out of scope (separate follow-ups): script/assertion error display, and response-body view modes (rendered HTML / preview). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/oc-docs/e2e/request-errors.spec.ts | 67 ++++++++++++++ .../ResponsePane/ResponsePane.tsx | 29 +++--- .../oc-docs/src/runner/RequestExecutor.ts | 34 +++---- .../src/runner/classifyRequestError.spec.ts | 82 +++++++++++++++++ .../src/runner/classifyRequestError.ts | 90 +++++++++++++++++++ packages/oc-docs/src/runner/index.ts | 2 + .../src/ui/ErrorBanner/ErrorBanner.tsx | 24 +++++ .../src/ui/ErrorBanner/StyledWrapper.tsx | 32 +++++++ 8 files changed, 322 insertions(+), 38 deletions(-) create mode 100644 packages/oc-docs/e2e/request-errors.spec.ts create mode 100644 packages/oc-docs/src/runner/classifyRequestError.spec.ts create mode 100644 packages/oc-docs/src/runner/classifyRequestError.ts create mode 100644 packages/oc-docs/src/ui/ErrorBanner/ErrorBanner.tsx create mode 100644 packages/oc-docs/src/ui/ErrorBanner/StyledWrapper.tsx diff --git a/packages/oc-docs/e2e/request-errors.spec.ts b/packages/oc-docs/e2e/request-errors.spec.ts new file mode 100644 index 0000000..4d498a0 --- /dev/null +++ b/packages/oc-docs/e2e/request-errors.spec.ts @@ -0,0 +1,67 @@ +import { test, expect, type Page } from '@playwright/test'; + +/** + * Locate an endpoint section by its h1 title (mirrors requests.spec.ts). + */ +function endpointSection(page: Page, name: string) { + return page.locator('.endpoint-section').filter({ + has: page.getByRole('heading', { name, level: 1, exact: true }), + }); +} + +/** Open the try-it playground for an endpoint and press SEND. */ +async function sendTryItRequest(page: Page, endpoint: string) { + await endpointSection(page, endpoint).getByRole('button', { name: 'Try' }).click(); + await page.getByRole('button', { name: 'SEND' }).click(); +} + +test.describe('Try-it request failure banner (BRU-3408)', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/'); + await page.waitForSelector('.endpoint-section'); + }); + + test('opaque network failure shows the contextual banner inside the Response tab', async ({ page }) => { + // fetch collapses CORS / connection-refused / DNS / TLS into one opaque + // failure. Aborting the request reproduces that exact state deterministically. + await page.route('**/api/users**', (route) => route.abort()); + + await sendTryItRequest(page, 'get users'); + + // Title + hedged message (never asserts CORS as certain) + next-step hint. + await expect(page.locator('.error-title')).toHaveText("Couldn't complete the request"); + await expect(page.locator('.error-message')).toContainText('almost always CORS'); + await expect(page.locator('.error-message')).toContainText('origin `null`'); + await expect(page.locator('.error-hint')).toContainText('run it from Bruno desktop'); + + // Banner renders inside the Response tab shell, not as a full-pane replacement. + await expect(page.getByRole('button', { name: 'Response', exact: true })).toBeVisible(); + }); + + test('does not mislabel the failure as a definite CORS error', async ({ page }) => { + await page.route('**/api/users**', (route) => route.abort()); + await sendTryItRequest(page, 'get users'); + + const message = await page.locator('.error-message').innerText(); + // Must hedge — the old code hardcoded "CORS error: ..." which is now forbidden. + expect(message.startsWith('CORS error')).toBe(false); + expect(message.toLowerCase()).toContain('the browser hides the exact cause'); + }); + + test('a 4xx response is NOT treated as a failure (renders normally)', async ({ page }) => { + await page.route('**/api/users**', (route) => + route.fulfill({ + status: 404, + headers: { 'access-control-allow-origin': '*' }, + contentType: 'application/json', + body: JSON.stringify({ error: 'not found' }), + }) + ); + + await sendTryItRequest(page, 'get users'); + + // No error banner; a 404 is a normal response (status shown, body rendered). + await expect(page.locator('.error-title')).toHaveCount(0); + await expect(page.getByText('404 Not Found')).toBeVisible(); + }); +}); diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx index 3297371..b7d5eba 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx @@ -1,5 +1,6 @@ import React, { useState } from 'react'; import Tabs from '../../../../../../ui/Tabs/Tabs'; +import ErrorBanner from '../../../../../../ui/ErrorBanner/ErrorBanner'; import { ResponseBodyTab, ResponseHeadersTab, TestResultsTab } from '../../Common'; interface ResponsePaneProps { @@ -46,20 +47,20 @@ const ResponsePane: React.FC = ({ response, isLoading }) => { ); } - if (response.error) { - return ( -
-
-
-

Request Failed

-

{response.error}

-
-
-
- ); - } + // A failed request (no HTTP response) renders a danger banner inside the + // Response tab, keeping the same tab shell as a successful response. + const renderErrorBanner = () => ( +
+ +
+ ); - const renderResponseBody = () => ; + const renderResponseBody = () => + response.error ? renderErrorBanner() : ; const renderHeaders = () => ; const renderTestResults = () => ( = ({ response, isLoading }) => { tabs={tabs} activeTab={activeTab} onTabChange={setActiveTab} - rightElement={statusInfo} + rightElement={response.error ? undefined : statusInfo} /> ); diff --git a/packages/oc-docs/src/runner/RequestExecutor.ts b/packages/oc-docs/src/runner/RequestExecutor.ts index 4e52326..1fea64c 100644 --- a/packages/oc-docs/src/runner/RequestExecutor.ts +++ b/packages/oc-docs/src/runner/RequestExecutor.ts @@ -1,14 +1,16 @@ import type { HttpRequest } from '@opencollection/types/requests/http'; import { RunRequestResponse } from './index'; import { getHttpMethod, getRequestUrl, getHttpHeaders, getHttpBody, getRequestAuth } from '../utils/schemaHelpers'; +import { classifyRequestError, DEFAULT_TIMEOUT_MS } from './classifyRequestError'; import stripJsonComments from 'strip-json-comments'; export class RequestExecutor { async executeRequest(request: HttpRequest, options: { timeout?: number } = {}): Promise { const startTime = Date.now(); + const timeoutMs = options.timeout ?? DEFAULT_TIMEOUT_MS; try { - const fetchOptions = await this.buildFetchOptions(request, options.timeout); + const fetchOptions = await this.buildFetchOptions(request, timeoutMs); const requestUrl = getRequestUrl(request); const response = await fetch(requestUrl, fetchOptions); const endTime = Date.now(); @@ -27,35 +29,19 @@ export class RequestExecutor { }; } catch (error) { const endTime = Date.now(); - let errorMessage = 'Request failed'; - let errorType = 'unknown'; - - if (error instanceof Error) { - errorMessage = error.message; - - // Categorize error types - if (error.name === 'AbortError' || error.message.includes('timeout')) { - errorType = 'timeout'; - errorMessage = `Request timed out after ${endTime - startTime}ms`; - } else if (error.name === 'TypeError' && error.message.includes('fetch')) { - errorType = 'cors'; - errorMessage = 'CORS error: Failed to fetch. The server either does not include the required CORS headers for this origin or cannot be reached.'; - } else if (error.message.includes('fetch')) { - errorType = 'network'; - } else if (error.message.includes('SSL') || error.message.includes('certificate')) { - errorType = 'ssl'; - } - } + const classified = classifyRequestError(error, { timeoutMs }); return { - error: errorMessage, - duration: endTime - startTime, - errorType + error: classified.message, + errorType: classified.type, + errorTitle: classified.title, + errorHint: classified.hint, + duration: endTime - startTime }; } } - private async buildFetchOptions(request: HttpRequest, timeout = 30000): Promise { + private async buildFetchOptions(request: HttpRequest, timeout = DEFAULT_TIMEOUT_MS): Promise { const method = getHttpMethod(request); const options: RequestInit = { method, diff --git a/packages/oc-docs/src/runner/classifyRequestError.spec.ts b/packages/oc-docs/src/runner/classifyRequestError.spec.ts new file mode 100644 index 0000000..55b2d04 --- /dev/null +++ b/packages/oc-docs/src/runner/classifyRequestError.spec.ts @@ -0,0 +1,82 @@ +import { describe, it, expect } from 'vitest'; +import { classifyRequestError } from './classifyRequestError'; + +// Helpers to build the exact error shapes the browser throws. +const timeoutError = () => { + const e = new Error('signal timed out'); + e.name = 'TimeoutError'; + return e; +}; + +const abortError = () => { + const e = new Error('The operation was aborted'); + e.name = 'AbortError'; + return e; +}; + +const failedToFetch = (message = 'Failed to fetch') => { + const e = new Error(message); + e.name = 'TypeError'; + return e; +}; + +describe('classifyRequestError', () => { + describe('timeout', () => { + it('classifies a TimeoutError from AbortSignal.timeout()', () => { + const result = classifyRequestError(timeoutError(), { timeoutMs: 30000 }); + expect(result.type).toBe('timeout'); + expect(result.title).toBe('Request timed out'); + expect(result.message).toBe('No response after 30s.'); + expect(result.hint).toBe('Check the host, or raise the timeout.'); + }); + + it('classifies a manual AbortError', () => { + expect(classifyRequestError(abortError()).type).toBe('timeout'); + }); + + it('phrases the message using the configured timeout', () => { + expect(classifyRequestError(timeoutError(), { timeoutMs: 5000 }).message).toBe('No response after 5s.'); + }); + + it('defaults to 30s when no timeout is provided', () => { + expect(classifyRequestError(timeoutError()).message).toBe('No response after 30s.'); + }); + }); + + describe('opaque', () => { + it('classifies Chrome\'s "Failed to fetch" as opaque (never asserts CORS as certain)', () => { + const result = classifyRequestError(failedToFetch(), { timeoutMs: 30000 }); + expect(result.type).toBe('opaque'); + expect(result.title).toBe("Couldn't complete the request"); + expect(result.message).toContain('almost always CORS'); + expect(result.message).toContain('origin `null`'); + // Must hedge, never assert CORS as definite. + expect(result.message).not.toMatch(/^CORS error/); + expect(result.hint).toBe('Allow CORS for this origin, serve the docs over http(s), or run it from Bruno desktop.'); + }); + + it('classifies Firefox\'s NetworkError as opaque', () => { + expect(classifyRequestError(failedToFetch('NetworkError when attempting to fetch resource')).type).toBe('opaque'); + }); + + it('classifies Safari\'s "Load failed" as opaque', () => { + expect(classifyRequestError(failedToFetch('Load failed')).type).toBe('opaque'); + }); + }); + + describe('uncategorized', () => { + it('surfaces the raw message for an unrecognized error', () => { + const result = classifyRequestError(new Error('Something weird happened')); + expect(result.type).toBe('uncategorized'); + expect(result.title).toBe("Couldn't complete the request"); + expect(result.message).toBe('Something weird happened'); + expect(result.hint).toBe('Check the request details and try again.'); + }); + + it('falls back to a generic message for a non-Error throw', () => { + const result = classifyRequestError('boom'); + expect(result.type).toBe('uncategorized'); + expect(result.message).toBe('The request could not be completed.'); + }); + }); +}); diff --git a/packages/oc-docs/src/runner/classifyRequestError.ts b/packages/oc-docs/src/runner/classifyRequestError.ts new file mode 100644 index 0000000..254f56e --- /dev/null +++ b/packages/oc-docs/src/runner/classifyRequestError.ts @@ -0,0 +1,90 @@ +/** + * Classifies a failed try-it request (one that never produced an HTTP response) + * into a user-facing error with a title, explanation, and one-line next step. + * + * The runner uses the browser `fetch`, which collapses CORS, DNS failures, + * connection-refused, offline, and TLS errors into a single opaque + * `TypeError: Failed to fetch` — the real cause lives only in devtools and is + * unreadable by the page. So only two states are actually distinguishable in + * code: a timeout (the abort signal fired) and everything-else-opaque. Anything + * we can't confirm is reported as uncategorized rather than guessed at. + * + * NOTE: 4xx/5xx responses are NOT failures — they never reach this function. + */ + +export type RequestErrorType = 'timeout' | 'opaque' | 'uncategorized'; + +export interface ClassifiedRequestError { + type: RequestErrorType; + title: string; + message: string; + hint: string; +} + +interface ClassifyOptions { + /** The request timeout in milliseconds, used to phrase the timeout message. */ + timeoutMs?: number; +} + +export const DEFAULT_TIMEOUT_MS = 30000; + +/** + * `AbortSignal.timeout()` rejects with a DOMException named `TimeoutError`. + * A manual `AbortController.abort()` rejects with one named `AbortError`. + * Older engines surface neither name cleanly, so we also sniff the message. + */ +const isTimeoutError = (error: unknown): boolean => { + if (!(error instanceof Error)) return false; + if (error.name === 'TimeoutError' || error.name === 'AbortError') return true; + const msg = error.message.toLowerCase(); + return msg.includes('timed out') || msg.includes('timeout'); +}; + +/** + * The browser's opaque network failure. `fetch` throws a `TypeError` whose + * message is "Failed to fetch" (Chrome) / "NetworkError when attempting to + * fetch resource" (Firefox) / "Load failed" (Safari). + */ +const isOpaqueFetchFailure = (error: unknown): boolean => { + if (!(error instanceof Error)) return false; + if (error.name !== 'TypeError') return false; + const msg = error.message.toLowerCase(); + return msg.includes('fetch') || msg.includes('load failed') || msg.includes('networkerror'); +}; + +export const classifyRequestError = ( + error: unknown, + options: ClassifyOptions = {} +): ClassifiedRequestError => { + const timeoutSeconds = Math.round((options.timeoutMs ?? DEFAULT_TIMEOUT_MS) / 1000); + + if (isTimeoutError(error)) { + return { + type: 'timeout', + title: 'Request timed out', + message: `No response after ${timeoutSeconds}s.`, + hint: 'Check the host, or raise the timeout.' + }; + } + + if (isOpaqueFetchFailure(error)) { + return { + type: 'opaque', + title: "Couldn't complete the request", + message: + "Blocked before reaching the server, almost always CORS: the API didn't allow this origin. " + + 'Docs opened from a file use origin `null`, which most servers reject. ' + + 'The browser hides the exact cause.', + hint: 'Allow CORS for this origin, serve the docs over http(s), or run it from Bruno desktop.' + }; + } + + // Anything else: surface the raw message rather than assert a cause we can't confirm. + const rawMessage = error instanceof Error && error.message ? error.message : 'The request could not be completed.'; + return { + type: 'uncategorized', + title: "Couldn't complete the request", + message: rawMessage, + hint: 'Check the request details and try again.' + }; +}; diff --git a/packages/oc-docs/src/runner/index.ts b/packages/oc-docs/src/runner/index.ts index 347b183..cf939f9 100644 --- a/packages/oc-docs/src/runner/index.ts +++ b/packages/oc-docs/src/runner/index.ts @@ -60,6 +60,8 @@ export interface RunRequestResponse { url?: string; error?: string; errorType?: string; + errorTitle?: string; + errorHint?: string; isCancel?: boolean; requestId?: string; assertionResults?: AssertionResultsResponse; diff --git a/packages/oc-docs/src/ui/ErrorBanner/ErrorBanner.tsx b/packages/oc-docs/src/ui/ErrorBanner/ErrorBanner.tsx new file mode 100644 index 0000000..56ebbd1 --- /dev/null +++ b/packages/oc-docs/src/ui/ErrorBanner/ErrorBanner.tsx @@ -0,0 +1,24 @@ +import React from 'react'; +import { StyledWrapper } from './StyledWrapper'; + +export interface ErrorBannerProps { + title: string; + message: string; + /** Optional one-line "what to do next" guidance shown beneath the message. */ + hint?: string; + className?: string; +} + +/** + * Danger banner for a failed try-it request: bold title, monospace message, + * and an optional next-step hint. Mirrors Bruno desktop's response error banner. + */ +const ErrorBanner: React.FC = ({ title, message, hint, className = '' }) => ( + +
{title}
+
{message}
+ {hint ?
{hint}
: null} + +); + +export default ErrorBanner; diff --git a/packages/oc-docs/src/ui/ErrorBanner/StyledWrapper.tsx b/packages/oc-docs/src/ui/ErrorBanner/StyledWrapper.tsx new file mode 100644 index 0000000..9637efc --- /dev/null +++ b/packages/oc-docs/src/ui/ErrorBanner/StyledWrapper.tsx @@ -0,0 +1,32 @@ +import styled from '@emotion/styled'; + +export const StyledWrapper = styled.div` + background-color: var(--bg-secondary); + border: 1px solid var(--border-color); + border-left: 4px solid var(--error-color); + border-radius: 6px; + padding: 1rem; + + .error-title { + font-weight: 600; + color: var(--error-color); + margin-bottom: 0.5rem; + } + + .error-message { + font-family: monospace; + font-size: 0.8125rem; + line-height: 1.3rem; + white-space: pre-wrap; + word-break: break-all; + color: var(--text-primary); + } + + .error-hint { + margin-top: 0.75rem; + font-size: 0.8125rem; + line-height: 1.3rem; + white-space: pre-wrap; + color: var(--text-secondary); + } +`; From f32a4ec8ece452203afa3c888887316672c8b531 Mon Sep 17 00:00:00 2001 From: Sundram Gupta Date: Wed, 10 Jun 2026 17:24:40 +0530 Subject: [PATCH 2/2] fix(oc-docs): classify try-it failures by request context (BRU-3408) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refine the failure classification per the updated ticket: instead of one opaque "almost always CORS" bucket, classify from the request context the browser does expose — whether it timed out, the page vs target scheme, and same-origin vs cross-origin / file. Five cases: - timeout -> "Request timed out. The server didn't respond in time." - mixed-content -> secure page (https) calling an insecure (http) URL - browser-blocked -> cross-origin, or docs opened from a file (CORS) - unreachable -> same-origin failure (server down / wrong URL) - unknown -> the underlying error message Same-vs-different is compared by ORIGIN (scheme+host+port), not site, since CORS is enforced per-origin. CORS is now suggested only for cross-origin or opened-from-file failures, never same-origin (AC #2). An unparseable request URL (e.g. an unresolved {{var}}) falls through to the underlying message. classifyRequestError now takes the resolved request URL and the page URL (window.location.href, passed in to keep it pure). The errorHint field is dropped — the new copy embeds the next step in the message. Response display, default timeout, and OAuth2 handling are unchanged. Tests reworked to cover all five cases (unit) and browser-blocked, same-origin-unreachable, and 4xx-not-a-failure (e2e). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/oc-docs/e2e/request-errors.spec.ts | 47 +++---- .../ResponsePane/ResponsePane.tsx | 1 - .../oc-docs/src/runner/RequestExecutor.ts | 7 +- .../src/runner/classifyRequestError.spec.ts | 86 ++++++++----- .../src/runner/classifyRequestError.ts | 118 +++++++++++++----- packages/oc-docs/src/runner/index.ts | 1 - 6 files changed, 174 insertions(+), 86 deletions(-) diff --git a/packages/oc-docs/e2e/request-errors.spec.ts b/packages/oc-docs/e2e/request-errors.spec.ts index 4d498a0..38cdcfe 100644 --- a/packages/oc-docs/e2e/request-errors.spec.ts +++ b/packages/oc-docs/e2e/request-errors.spec.ts @@ -9,43 +9,45 @@ function endpointSection(page: Page, name: string) { }); } -/** Open the try-it playground for an endpoint and press SEND. */ -async function sendTryItRequest(page: Page, endpoint: string) { +/** Open the try-it playground for an endpoint. */ +async function openTryIt(page: Page, endpoint: string) { await endpointSection(page, endpoint).getByRole('button', { name: 'Try' }).click(); - await page.getByRole('button', { name: 'SEND' }).click(); } -test.describe('Try-it request failure banner (BRU-3408)', () => { +test.describe('Try-it request failure messages (BRU-3408)', () => { test.beforeEach(async ({ page }) => { await page.goto('/'); await page.waitForSelector('.endpoint-section'); }); - test('opaque network failure shows the contextual banner inside the Response tab', async ({ page }) => { - // fetch collapses CORS / connection-refused / DNS / TLS into one opaque - // failure. Aborting the request reproduces that exact state deterministically. + test('cross-origin failure is classified as browser-blocked (CORS), inside the Response tab', async ({ page }) => { + // The sample collection targets localhost:8081 — a different origin than the + // docs page (127.0.0.1:3001). Aborting reproduces the opaque failure. await page.route('**/api/users**', (route) => route.abort()); - await sendTryItRequest(page, 'get users'); + await openTryIt(page, 'get users'); + await page.getByRole('button', { name: 'SEND' }).click(); - // Title + hedged message (never asserts CORS as certain) + next-step hint. - await expect(page.locator('.error-title')).toHaveText("Couldn't complete the request"); - await expect(page.locator('.error-message')).toContainText('almost always CORS'); - await expect(page.locator('.error-message')).toContainText('origin `null`'); - await expect(page.locator('.error-hint')).toContainText('run it from Bruno desktop'); + await expect(page.locator('.error-title')).toHaveText('Request blocked'); + await expect(page.locator('.error-message')).toContainText('usually CORS'); + await expect(page.locator('.error-message')).toContainText('Bruno desktop app'); // Banner renders inside the Response tab shell, not as a full-pane replacement. await expect(page.getByRole('button', { name: 'Response', exact: true })).toBeVisible(); }); - test('does not mislabel the failure as a definite CORS error', async ({ page }) => { - await page.route('**/api/users**', (route) => route.abort()); - await sendTryItRequest(page, 'get users'); + test('same-origin failure is "unreachable" and never mentions CORS', async ({ page }) => { + // Point the request at the docs page's own origin, then fail it. + await page.route('**/same-origin-fail**', (route) => route.abort()); + + await openTryIt(page, 'get users'); + await page.getByPlaceholder('Enter request URL').fill('http://127.0.0.1:3001/same-origin-fail'); + await page.getByRole('button', { name: 'SEND' }).click(); - const message = await page.locator('.error-message').innerText(); - // Must hedge — the old code hardcoded "CORS error: ..." which is now forbidden. - expect(message.startsWith('CORS error')).toBe(false); - expect(message.toLowerCase()).toContain('the browser hides the exact cause'); + await expect(page.locator('.error-title')).toHaveText("Couldn't reach the server"); + const message = (await page.locator('.error-message').innerText()).toLowerCase(); + expect(message).toContain('may be down'); + expect(message).not.toContain('cors'); }); test('a 4xx response is NOT treated as a failure (renders normally)', async ({ page }) => { @@ -58,9 +60,10 @@ test.describe('Try-it request failure banner (BRU-3408)', () => { }) ); - await sendTryItRequest(page, 'get users'); + await openTryIt(page, 'get users'); + await page.getByRole('button', { name: 'SEND' }).click(); - // No error banner; a 404 is a normal response (status shown, body rendered). + // No error banner; a 404 is a normal response (status shown). await expect(page.locator('.error-title')).toHaveCount(0); await expect(page.getByText('404 Not Found')).toBeVisible(); }); diff --git a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx index b7d5eba..c01d0b5 100644 --- a/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx +++ b/packages/oc-docs/src/components/PlaygroundDrawer/DrawerContent/Views/PlaygroundView/ResponsePane/ResponsePane.tsx @@ -54,7 +54,6 @@ const ResponsePane: React.FC = ({ response, isLoading }) => { ); diff --git a/packages/oc-docs/src/runner/RequestExecutor.ts b/packages/oc-docs/src/runner/RequestExecutor.ts index 1fea64c..d938db6 100644 --- a/packages/oc-docs/src/runner/RequestExecutor.ts +++ b/packages/oc-docs/src/runner/RequestExecutor.ts @@ -29,13 +29,16 @@ export class RequestExecutor { }; } catch (error) { const endTime = Date.now(); - const classified = classifyRequestError(error, { timeoutMs }); + const classified = classifyRequestError(error, { + timeoutMs, + requestUrl: getRequestUrl(request), + pageUrl: typeof window !== 'undefined' ? window.location.href : undefined + }); return { error: classified.message, errorType: classified.type, errorTitle: classified.title, - errorHint: classified.hint, duration: endTime - startTime }; } diff --git a/packages/oc-docs/src/runner/classifyRequestError.spec.ts b/packages/oc-docs/src/runner/classifyRequestError.spec.ts index 55b2d04..e3be91f 100644 --- a/packages/oc-docs/src/runner/classifyRequestError.spec.ts +++ b/packages/oc-docs/src/runner/classifyRequestError.spec.ts @@ -23,59 +23,89 @@ const failedToFetch = (message = 'Failed to fetch') => { describe('classifyRequestError', () => { describe('timeout', () => { it('classifies a TimeoutError from AbortSignal.timeout()', () => { - const result = classifyRequestError(timeoutError(), { timeoutMs: 30000 }); + const result = classifyRequestError(timeoutError()); expect(result.type).toBe('timeout'); expect(result.title).toBe('Request timed out'); - expect(result.message).toBe('No response after 30s.'); - expect(result.hint).toBe('Check the host, or raise the timeout.'); + expect(result.message).toBe("Request timed out. The server didn't respond in time."); }); it('classifies a manual AbortError', () => { expect(classifyRequestError(abortError()).type).toBe('timeout'); }); + }); - it('phrases the message using the configured timeout', () => { - expect(classifyRequestError(timeoutError(), { timeoutMs: 5000 }).message).toBe('No response after 5s.'); + describe('mixed content (secure page, insecure URL)', () => { + it('classifies an https page calling an http URL', () => { + const result = classifyRequestError(failedToFetch(), { + pageUrl: 'https://docs.example.com/api.html', + requestUrl: 'http://api.example.com/users' + }); + expect(result.type).toBe('mixed-content'); + expect(result.message).toContain('secure (https)'); + expect(result.message).toContain('insecure (http)'); }); + }); - it('defaults to 30s when no timeout is provided', () => { - expect(classifyRequestError(timeoutError()).message).toBe('No response after 30s.'); + describe('browser-blocked (CORS — cross-origin or opened from a file)', () => { + it('classifies a cross-origin request (different host, same site)', () => { + const result = classifyRequestError(failedToFetch(), { + pageUrl: 'https://docs.example.com/', + requestUrl: 'https://api.example.com/users' + }); + expect(result.type).toBe('browser-blocked'); + expect(result.message).toContain('usually CORS'); + expect(result.message).toContain('Bruno desktop app'); }); - }); - describe('opaque', () => { - it('classifies Chrome\'s "Failed to fetch" as opaque (never asserts CORS as certain)', () => { - const result = classifyRequestError(failedToFetch(), { timeoutMs: 30000 }); - expect(result.type).toBe('opaque'); - expect(result.title).toBe("Couldn't complete the request"); - expect(result.message).toContain('almost always CORS'); - expect(result.message).toContain('origin `null`'); - // Must hedge, never assert CORS as definite. - expect(result.message).not.toMatch(/^CORS error/); - expect(result.hint).toBe('Allow CORS for this origin, serve the docs over http(s), or run it from Bruno desktop.'); + it('classifies a request from a docs page opened from a file (origin null)', () => { + const result = classifyRequestError(failedToFetch(), { + pageUrl: 'file:///Users/me/docs.html', + requestUrl: 'https://api.example.com/users' + }); + expect(result.type).toBe('browser-blocked'); }); - it('classifies Firefox\'s NetworkError as opaque', () => { - expect(classifyRequestError(failedToFetch('NetworkError when attempting to fetch resource')).type).toBe('opaque'); + it('never suggests CORS for a same-origin failure', () => { + const result = classifyRequestError(failedToFetch(), { + pageUrl: 'https://app.example.com/docs', + requestUrl: 'https://app.example.com/api/users' + }); + expect(result.type).not.toBe('browser-blocked'); + expect(result.message.toLowerCase()).not.toContain('cors'); }); + }); - it('classifies Safari\'s "Load failed" as opaque', () => { - expect(classifyRequestError(failedToFetch('Load failed')).type).toBe('opaque'); + describe('server unreachable (same origin)', () => { + it('classifies a same-origin failure as unreachable', () => { + const result = classifyRequestError(failedToFetch(), { + pageUrl: 'https://app.example.com/docs', + requestUrl: 'https://app.example.com/api/users' + }); + expect(result.type).toBe('unreachable'); + expect(result.message).toBe("Couldn't reach the server. It may be down, or the URL may be wrong."); }); }); - describe('uncategorized', () => { - it('surfaces the raw message for an unrecognized error', () => { + describe('anything else -> underlying message', () => { + it('surfaces the raw message for a non-network error', () => { const result = classifyRequestError(new Error('Something weird happened')); - expect(result.type).toBe('uncategorized'); - expect(result.title).toBe("Couldn't complete the request"); + expect(result.type).toBe('unknown'); expect(result.message).toBe('Something weird happened'); - expect(result.hint).toBe('Check the request details and try again.'); + }); + + it('falls through to the raw message when the request URL is unparseable', () => { + // e.g. an unresolved {{baseUrl}} leaves a relative path with no origin to compare. + const result = classifyRequestError(failedToFetch('Failed to fetch'), { + pageUrl: 'https://docs.example.com/', + requestUrl: '/users' + }); + expect(result.type).toBe('unknown'); + expect(result.message).toBe('Failed to fetch'); }); it('falls back to a generic message for a non-Error throw', () => { const result = classifyRequestError('boom'); - expect(result.type).toBe('uncategorized'); + expect(result.type).toBe('unknown'); expect(result.message).toBe('The request could not be completed.'); }); }); diff --git a/packages/oc-docs/src/runner/classifyRequestError.ts b/packages/oc-docs/src/runner/classifyRequestError.ts index 254f56e..8bfccfa 100644 --- a/packages/oc-docs/src/runner/classifyRequestError.ts +++ b/packages/oc-docs/src/runner/classifyRequestError.ts @@ -1,29 +1,41 @@ /** * Classifies a failed try-it request (one that never produced an HTTP response) - * into a user-facing error with a title, explanation, and one-line next step. + * into a user-facing title + message. * - * The runner uses the browser `fetch`, which collapses CORS, DNS failures, - * connection-refused, offline, and TLS errors into a single opaque - * `TypeError: Failed to fetch` — the real cause lives only in devtools and is - * unreadable by the page. So only two states are actually distinguishable in - * code: a timeout (the abort signal fired) and everything-else-opaque. Anything - * we can't confirm is reported as uncategorized rather than guessed at. + * Browser `fetch` collapses CORS, DNS, connection-refused, offline, and TLS into + * one opaque failure with no detail — the real cause lives only in devtools and + * is unreadable by the page. So we classify from the REQUEST CONTEXT (did it time + * out, the page vs target scheme, and same-origin vs cross-origin / file) rather + * than from the error text. + * + * Origin (scheme + host + port), not site, is what decides cross-origin: a request + * from https://docs.example.com to https://api.example.com is cross-origin and + * triggers CORS even though both share the site example.com. Comparing by origin + * matches how the browser actually enforces CORS. * * NOTE: 4xx/5xx responses are NOT failures — they never reach this function. */ -export type RequestErrorType = 'timeout' | 'opaque' | 'uncategorized'; +export type RequestErrorType = + | 'timeout' + | 'mixed-content' + | 'browser-blocked' + | 'unreachable' + | 'unknown'; export interface ClassifiedRequestError { type: RequestErrorType; title: string; message: string; - hint: string; } interface ClassifyOptions { - /** The request timeout in milliseconds, used to phrase the timeout message. */ + /** The request timeout in milliseconds (reserved for future use / parity). */ timeoutMs?: number; + /** The fully-resolved request URL passed to fetch (after variable interpolation). */ + requestUrl?: string; + /** The page URL the docs are running on, typically window.location.href. */ + pageUrl?: string; } export const DEFAULT_TIMEOUT_MS = 30000; @@ -52,39 +64,81 @@ const isOpaqueFetchFailure = (error: unknown): boolean => { return msg.includes('fetch') || msg.includes('load failed') || msg.includes('networkerror'); }; +const safeParseUrl = (url?: string): URL | null => { + if (!url) return null; + try { + return new URL(url); + } catch { + return null; + } +}; + +const TIMEOUT: ClassifiedRequestError = { + type: 'timeout', + title: 'Request timed out', + message: "Request timed out. The server didn't respond in time." +}; + +const MIXED_CONTENT: ClassifiedRequestError = { + type: 'mixed-content', + title: 'Request blocked', + message: + '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.' +}; + +const BROWSER_BLOCKED: ClassifiedRequestError = { + type: 'browser-blocked', + title: 'Request blocked', + message: + "Request blocked by your browser, usually CORS: the API didn't allow requests " + + 'from this page. Try it in the Bruno desktop app.' +}; + +const UNREACHABLE: ClassifiedRequestError = { + type: 'unreachable', + title: "Couldn't reach the server", + message: "Couldn't reach the server. It may be down, or the URL may be wrong." +}; + export const classifyRequestError = ( error: unknown, options: ClassifyOptions = {} ): ClassifiedRequestError => { - const timeoutSeconds = Math.round((options.timeoutMs ?? DEFAULT_TIMEOUT_MS) / 1000); - if (isTimeoutError(error)) { - return { - type: 'timeout', - title: 'Request timed out', - message: `No response after ${timeoutSeconds}s.`, - hint: 'Check the host, or raise the timeout.' - }; + return TIMEOUT; } if (isOpaqueFetchFailure(error)) { - return { - type: 'opaque', - title: "Couldn't complete the request", - message: - "Blocked before reaching the server, almost always CORS: the API didn't allow this origin. " + - 'Docs opened from a file use origin `null`, which most servers reject. ' + - 'The browser hides the exact cause.', - hint: 'Allow CORS for this origin, serve the docs over http(s), or run it from Bruno desktop.' - }; + const target = safeParseUrl(options.requestUrl); + const page = safeParseUrl(options.pageUrl); + + // Without a parseable target URL we can't reason about scheme/origin, so we + // fall through to the underlying message rather than guess. + if (target && page) { + // Secure page requesting an insecure URL -> the browser blocks it as mixed content. + if (page.protocol === 'https:' && target.protocol === 'http:') { + return MIXED_CONTENT; + } + + // Docs opened from a file have origin "null"; any cross-origin request is + // subject to CORS. Same-origin failures can't be CORS, so the server is + // unreachable (down, or wrong URL). + const openedFromFile = page.origin === 'null' || page.protocol === 'file:'; + if (openedFromFile || target.origin !== page.origin) { + return BROWSER_BLOCKED; + } + + return UNREACHABLE; + } } - // Anything else: surface the raw message rather than assert a cause we can't confirm. - const rawMessage = error instanceof Error && error.message ? error.message : 'The request could not be completed.'; + // Anything else (or an unparseable URL): surface the underlying error message. + const rawMessage = + error instanceof Error && error.message ? error.message : 'The request could not be completed.'; return { - type: 'uncategorized', + type: 'unknown', title: "Couldn't complete the request", - message: rawMessage, - hint: 'Check the request details and try again.' + message: rawMessage }; }; diff --git a/packages/oc-docs/src/runner/index.ts b/packages/oc-docs/src/runner/index.ts index cf939f9..2de5c8f 100644 --- a/packages/oc-docs/src/runner/index.ts +++ b/packages/oc-docs/src/runner/index.ts @@ -61,7 +61,6 @@ export interface RunRequestResponse { error?: string; errorType?: string; errorTitle?: string; - errorHint?: string; isCancel?: boolean; requestId?: string; assertionResults?: AssertionResultsResponse;