Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions packages/cli/src/bridge-errors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
classifyBridgeError,
FetchproxyScopeError,
FetchproxyHintedError,
FetchproxySessionNotReadyError,
} from '@fetchproxy/server';
import { EXIT, UsageError, type Io } from './output.js';
Expand Down Expand Up @@ -32,15 +32,18 @@ export function mapBridgeError(err: unknown, io: Io): number {
);
return EXIT.USAGE;
}
// A widened declared scope is rejected by the extension's gate #2 until the
// user re-approves. That arrives as a `protocol` error, so without this it
// inherits the blanket "version mismatch — update both" hint below and sends
// people chasing a version problem that does not exist.
// Some rejections know their own remedy — a widened scope needs a re-pair,
// a missing tab needs a tab. Both arrive as `protocol` errors, so without
// this they inherit the blanket "version mismatch — update both" hint below
// and send people chasing a version problem that does not exist.
//
// The wording knowledge now lives on the error itself (FetchproxyScopeError,
// server 1.10+) rather than in a regex here — the CLI is not the only
// consumer that needs it, and a second copy would drift from the first.
if (err instanceof FetchproxyScopeError) {
// The wording knowledge lives on the error itself (server 1.10+) rather than
// in a regex here — the CLI is not the only consumer that needs it, and a
// second copy would drift from the first. Branching on the shared
// FetchproxyHintedError base rather than each subclass (1.12+, #204) means
// the next hinted error is rendered right here without a new branch; keying
// on FetchproxyScopeError alone is how the no-tab case ended up mis-hinted.
if (err instanceof FetchproxyHintedError) {
io.err(`bridge error (${kind}): ${err.originalError} — ${err.hint}`);
return EXIT.BRIDGE;
}
Expand Down
36 changes: 36 additions & 0 deletions packages/cli/tests/bridge-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,39 @@ describe('mapBridgeError — scope-diff errors are not version mismatches', () =
expect(io.errs.join('\n')).toMatch(/version mismatch/i);
});
});

describe('mapBridgeError — a missing tab is not a version mismatch', () => {
// Reported as #204: `fpx post-json https://api.creditkarma.com/graphql` on a
// current CLI and a current extension printed
// bridge error (protocol): no tab matching https://api.creditkarma.com/
// — extension/server version mismatch — update both.
// Nothing was mismatched; nothing was open on that host.
it('tells the user to open a tab, not to update', () => {
const io = memIo();
const code = mapBridgeError(
protocolErrorFrom('no tab matching https://api.creditkarma.com/'),
io,
);
expect(code).toBe(EXIT.BRIDGE);
const out = io.errs.join('\n');
expect(out).toMatch(/no tab matching https:\/\/api\.creditkarma\.com\//);
expect(out).toMatch(/open a tab/i);
expect(out).not.toMatch(/version mismatch/i);
expect(out).not.toMatch(/update both/i);
});

it('leaves the unreachable-content-script wording to say its own piece', () => {
// That message already tells the user to refresh the page. "Open a tab"
// would be wrong there — one is open.
const io = memIo();
mapBridgeError(
protocolErrorFrom(
'no tab matching https://x.com/ has the fetchproxy content script loaded ' +
'(1 URL match, none responded). Refresh the page in your browser to inject ' +
'the content script, then retry.',
),
io,
);
expect(io.errs.join('\n')).toMatch(/Refresh the page/);
});
});
2 changes: 2 additions & 0 deletions packages/server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ export {
FetchproxyProtocolError,
FetchproxyHttpError,
FetchproxyBridgeDownError,
FetchproxyHintedError,
FetchproxyScopeError,
FetchproxyNoTabError,
protocolErrorFrom,
FetchproxyTimeoutError,
} from './ws-server.js';
Expand Down
94 changes: 77 additions & 17 deletions packages/server/src/ws-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,34 @@ export class FetchproxyBridgeDownError extends FetchproxyProtocolError {
}
}

/**
* 1.12.0+: a protocol error that knows its own remedy.
*
* Every consumer that re-wraps bridge errors — the CLI, `@fetchproxy/bootstrap`
* callers, MCPs with their own auth copy — needs to answer "and what do I do
* about it?". Answering per-consumer is how the guidance goes missing, and
* answering per-subclass is how consumers end up branching on
* `instanceof FetchproxyScopeError` and inheriting blanket advice for
* everything else (the bug behind #204).
*
* So the shape lives here: catch `FetchproxyHintedError`, render
* `originalError — hint`, and every present and future hinted error renders
* correctly without a new branch.
*/
export class FetchproxyHintedError extends FetchproxyProtocolError {
/** The extension's raw rejection, unmodified. */
readonly originalError: string;
/** What the user should actually do, in prose. */
readonly hint: string;

constructor(originalError: string, hint: string) {
super(`${originalError} — ${hint}`);
this.name = 'FetchproxyHintedError';
this.originalError = originalError;
this.hint = hint;
}
}

/**
* 1.10.0+: the extension rejected a request because its declared scope no
* longer covers what was asked for (gate #2).
Expand All @@ -508,21 +536,42 @@ export class FetchproxyBridgeDownError extends FetchproxyProtocolError {
* guidance on the error itself means every consumer can surface it the way
* they already surface {@link FetchproxyBridgeDownError.hint}.
*/
export class FetchproxyScopeError extends FetchproxyProtocolError {
/** The extension's raw rejection, unmodified. */
readonly originalError: string;
readonly hint: string;

export class FetchproxyScopeError extends FetchproxyHintedError {
constructor(originalError: string) {
const hint =
super(
originalError,
'the declared scope changed since you paired, so the extension is ' +
'refusing the request. Revoke this MCP in the Transporter extension ' +
'popup, then re-run — you will be asked to approve the new scope. ' +
'This is not a version problem and does not need an update.';
super(`${originalError} — ${hint}`);
'refusing the request. Revoke this MCP in the Transporter extension ' +
'popup, then re-run — you will be asked to approve the new scope. ' +
'This is not a version problem and does not need an update.',
);
this.name = 'FetchproxyScopeError';
this.originalError = originalError;
this.hint = hint;
}
}

/**
* 1.12.0+: no browser tab is open on the host the request needed.
*
* Typed for the same reason as {@link FetchproxyScopeError}: untyped, the
* rejection is a plain `FetchproxyProtocolError`, `classifyBridgeError`
* (which dispatches on type, not message) calls it `protocol`, and every
* consumer's blanket protocol advice lands on it. In the CLI that advice was
* "extension/server version mismatch — update both", so a user whose versions
* were entirely current got sent to update them (#204).
*
* Deliberately NOT applied to the "matched a tab, but its content script never
* answered" wording. That has a different remedy — refresh the page rather
* than open one — and the extension's own message already spells it out, so
* retyping it here would staple contradictory advice onto it.
*/
export class FetchproxyNoTabError extends FetchproxyHintedError {
constructor(originalError: string) {
super(
originalError,
'open a tab on that host and sign in, then re-run. This is not a ' +
'version problem and does not need an update.',
);
this.name = 'FetchproxyNoTabError';
}
}

Expand All @@ -537,17 +586,28 @@ export class FetchproxyScopeError extends FetchproxyProtocolError {
*/
const SCOPE_REJECTION = /not in declared/;

/**
* "Nothing is open on that host."
*
* The negative lookahead keeps the sibling wording out: `no tab matching <url>
* has the fetchproxy content script loaded …` means a tab DID match but never
* answered, which is fixed by refreshing the page, not by opening one. Both
* start `no tab matching `, so matching that prefix alone would give the
* unreachable-content-script case advice that cannot fix it.
*/
const NO_TAB_REJECTION = /no tab matching (?!.*content script loaded)/;

/**
* Build the right error for an extension rejection.
*
* Use this instead of `new FetchproxyProtocolError(err)` at every site that
* turns an `ok:false` response into a throw, so scope rejections cannot
* silently lose their guidance again at one forgotten call site.
* turns an `ok:false` response into a throw, so rejections that know their own
* remedy cannot silently lose it again at one forgotten call site.
*/
export function protocolErrorFrom(error: string): FetchproxyProtocolError {
return SCOPE_REJECTION.test(error)
? new FetchproxyScopeError(error)
: new FetchproxyProtocolError(error);
if (SCOPE_REJECTION.test(error)) return new FetchproxyScopeError(error);
if (NO_TAB_REJECTION.test(error)) return new FetchproxyNoTabError(error);
return new FetchproxyProtocolError(error);
}

/**
Expand Down
99 changes: 99 additions & 0 deletions packages/server/tests/no-tab-error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { describe, it, expect } from 'vitest';
import {
FetchproxyNoTabError,
FetchproxyScopeError,
FetchproxyHintedError,
FetchproxyProtocolError,
protocolErrorFrom,
classifyBridgeError,
} from '../src/index.js';

/**
* "no tab matching <url>" means exactly what it says: nothing is open on that
* host. But `classifyBridgeError` is type-based, so the error arrived as a
* plain `FetchproxyProtocolError` and the CLI's hint table gave every
* `protocol` error the blanket "extension/server version mismatch — update
* both." Real report (#204):
*
* bridge error (protocol): no tab matching https://api.creditkarma.com/
* — extension/server version mismatch — update both.
*
* Both were current. There was no mismatch. This is the same misdirection
* FetchproxyScopeError was introduced to stop, so it gets the same treatment:
* the guidance rides on the error, where every consumer can reach it.
*/
describe('protocolErrorFrom — no-tab rejections', () => {
it('types a bare no-tab rejection as a no-tab error', () => {
const err = protocolErrorFrom('no tab matching https://api.creditkarma.com/');
expect(err).toBeInstanceOf(FetchproxyNoTabError);
});

it('names the remedy — open a tab — and not a version bump', () => {
const err = protocolErrorFrom('no tab matching https://x.com/') as FetchproxyNoTabError;
expect(err.hint).toMatch(/open a tab/i);
// The whole point: it must not read as a version problem, which is where
// the blanket `protocol` hint sent people.
expect(err.hint).not.toMatch(/version mismatch/i);
expect(err.hint).not.toMatch(/update both/i);
});

it('preserves the raw extension error for callers that want it', () => {
const err = protocolErrorFrom('no tab matching https://x.com/') as FetchproxyNoTabError;
expect(err.originalError).toBe('no tab matching https://x.com/');
});

it('is a FetchproxyProtocolError, so existing catch sites still match', () => {
const err = protocolErrorFrom('no tab matching https://x.com/');
expect(err).toBeInstanceOf(FetchproxyProtocolError);
expect(classifyBridgeError(err)).toBe('protocol');
});

it('leaves the content-script variant alone — it has a different remedy', () => {
// "matched but unreachable" is fixed by refreshing the page, not by
// opening one, and the extension's own wording already says so. Retyping
// it here would bolt on contradictory advice.
const msg =
'no tab matching https://x.com/ has the fetchproxy content script loaded ' +
'(1 URL match, none responded). Refresh the page in your browser to inject ' +
'the content script, then retry.';
const err = protocolErrorFrom(msg);
expect(err).not.toBeInstanceOf(FetchproxyNoTabError);
expect(err).toBeInstanceOf(FetchproxyProtocolError);
});

it('leaves unrelated protocol errors alone', () => {
const err = protocolErrorFrom('unknown frame type "wat"');
expect(err).not.toBeInstanceOf(FetchproxyNoTabError);
expect(err).toBeInstanceOf(FetchproxyProtocolError);
});

it('does not collide with scope rejections', () => {
const scope = protocolErrorFrom('cookie keys not in declared set: a');
expect(scope).toBeInstanceOf(FetchproxyScopeError);
expect(scope).not.toBeInstanceOf(FetchproxyNoTabError);
});
});

/**
* Both hinted errors carry the same shape. Consumers — the CLI included —
* should be able to render "raw error — remedy" without knowing which
* subclass they caught, so the next hinted error added doesn't need a new
* branch at every call site to avoid inheriting the wrong blanket advice.
*/
describe('FetchproxyHintedError — the shared contract', () => {
it.each([
['scope', 'cookie keys not in declared set: a'],
['no-tab', 'no tab matching https://x.com/'],
])('%s rejections are hinted errors', (_label, msg) => {
const err = protocolErrorFrom(msg);
expect(err).toBeInstanceOf(FetchproxyHintedError);
const hinted = err as FetchproxyHintedError;
expect(hinted.originalError).toBe(msg);
expect(hinted.hint.length).toBeGreaterThan(0);
expect(hinted.message).toContain(msg);
});

it('does not tag ordinary protocol errors as hinted', () => {
expect(protocolErrorFrom('unknown frame type "wat"')).not.toBeInstanceOf(FetchproxyHintedError);
});
});