Browse Source

fix: address codex review round 2

- Preserve abort errors while parsing search responses: when the caller's
  AbortSignal fires after headers but during response.json() (both the success
  and HTTP-error body parses), surface WEB_ABORTED instead of wrapping it as
  WEB_PROVIDER_ERROR, so agent cancel/dispose is not misreported as a provider
  failure. Applied to both the Exa and Perplexity providers.
- Report a malformed baseURL as misconfigured in status() (URL.canParse), so
  selection diagnostics and execution agree (configured-unavailable up front
  rather than a late WEB_PROVIDER_ERROR). WebProviderStatus already had the
  reason.
Dudu-0223 3 months ago
parent
commit
0930e483ec

+ 1 - 1
packages/web/web-search-exa/README.md

@@ -9,7 +9,7 @@ This is an **implementation** package: it registers a provider into `ctx.web`, i
 | Key | Default | Meaning |
 |---|---|---|
 | `apiKey` | `$EXA_API_KEY` | Exa API key. Empty/absent → provider `status()` reports `missing-credential` (the seam reports `configured-unavailable`/`none`). |
-| `baseURL` | `https://api.exa.ai` | Endpoint base; `/search` is appended. |
+| `baseURL` | `https://api.exa.ai` | Endpoint base; `/search` is appended. An unparseable value makes `status()` report `misconfigured`. |
 
 ```yaml
 - id: web-search-exa

+ 15 - 5
packages/web/web-search-exa/src/provider.ts

@@ -72,6 +72,7 @@ export class ExaSearchProvider implements WebSearchProvider {
 
   status(): WebProviderStatus {
     if (this.options.apiKey.length === 0) return { available: false, reason: 'missing-credential' }
+    if (!isValidBaseUrl(this.options.baseURL)) return { available: false, reason: 'misconfigured' }
     return { available: true }
   }
 
@@ -105,11 +106,14 @@ export class ExaSearchProvider implements WebSearchProvider {
         const parsed = await response.json() as ExaError
         const detail = parsed.error ?? parsed.message
         if (detail !== undefined && detail.length > 0) message = detail
-      } catch {
-        // The HTTP status is already captured in `message` above; a malformed or
-        // non-JSON error body (normal for gateway 5xx/429s) can only cost a
-        // richer provider message, never the real error. `response.json()` is
-        // the sole statement and nothing else of consequence reaches here.
+      } catch (error: unknown) {
+        // An abort fired mid-body must surface as WEB_ABORTED, not be swallowed
+        // into a generic HTTP-error message — cancellation is not a provider
+        // error (the seam's cancellation contract).
+        if (isAbortError(error)) throw new WebError('Exa search aborted', 'WEB_ABORTED', { cause: error })
+        // Otherwise: the HTTP status is already captured in `message` above; a
+        // malformed/non-JSON error body (normal for gateway 5xx/429s) can only
+        // cost a richer provider message, never the real error.
       }
       throw new WebError(message, 'WEB_PROVIDER_ERROR')
     }
@@ -118,12 +122,18 @@ export class ExaSearchProvider implements WebSearchProvider {
     try {
       payload = await response.json() as ExaSearchResponse
     } catch (error: unknown) {
+      if (isAbortError(error)) throw new WebError('Exa search aborted', 'WEB_ABORTED', { cause: error })
       throw new WebError(`Exa returned an unparseable response body: ${String(error)}`, 'WEB_PROVIDER_ERROR', { cause: error })
     }
     return mapExaResponse(request.query, payload)
   }
 }
 
+/** True when `baseURL` parses as an absolute URL (a cheap local config check). */
+function isValidBaseUrl(baseURL: string): boolean {
+  return URL.canParse(baseURL)
+}
+
 /** True for a fetch/`AbortSignal` abort, surfaced as `WEB_ABORTED`. */
 function isAbortError(error: unknown): boolean {
   return error instanceof DOMException && error.name === 'AbortError'

+ 19 - 0
packages/web/web-search-exa/tests/exa.spec.ts

@@ -71,6 +71,11 @@ describe('ExaSearchProvider status', () => {
   it('is available with a key', () => {
     expect(new ExaSearchProvider(options).status()).toEqual({ available: true })
   })
+
+  it('is misconfigured when the base URL is unparseable', () => {
+    expect(new ExaSearchProvider({ apiKey: 'exa-key', baseURL: 'not a url' }).status())
+      .toEqual({ available: false, reason: 'misconfigured' })
+  })
 })
 
 describe('ExaSearchProvider request mapping', () => {
@@ -142,6 +147,20 @@ describe('ExaSearchProvider error handling', () => {
     await expect(new ExaSearchProvider(options).search({ query: 'q' }))
       .rejects.toThrow(expect.objectContaining({ code: 'WEB_PROVIDER_ERROR' }))
   })
+
+  it('surfaces an abort during success-body parse as WEB_ABORTED, not provider error', async () => {
+    const body = { json: () => Promise.reject(new DOMException('aborted', 'AbortError')), ok: true, status: 200 }
+    vi.stubGlobal('fetch', vi.fn(async () => body as unknown as Response))
+    await expect(new ExaSearchProvider(options).search({ query: 'q' }))
+      .rejects.toThrow(expect.objectContaining({ code: 'WEB_ABORTED' }))
+  })
+
+  it('surfaces an abort during error-body parse as WEB_ABORTED', async () => {
+    const body = { json: () => Promise.reject(new DOMException('aborted', 'AbortError')), ok: false, status: 500 }
+    vi.stubGlobal('fetch', vi.fn(async () => body as unknown as Response))
+    await expect(new ExaSearchProvider(options).search({ query: 'q' }))
+      .rejects.toThrow(expect.objectContaining({ code: 'WEB_ABORTED' }))
+  })
 })
 
 describe('web-search-exa plugin registration', () => {

+ 1 - 1
packages/web/web-search-perplexity/README.md

@@ -9,7 +9,7 @@ This is an **implementation** package: it registers a provider into `ctx.web`, i
 | Key | Default | Meaning |
 |---|---|---|
 | `apiKey` | `$PERPLEXITY_API_KEY` | Perplexity API key. Empty/absent → provider `status()` reports `missing-credential`. |
-| `baseURL` | `https://api.perplexity.ai` | Endpoint base; `/chat/completions` is appended. |
+| `baseURL` | `https://api.perplexity.ai` | Endpoint base; `/chat/completions` is appended. An unparseable value makes `status()` report `misconfigured`. |
 | `model` | `sonar` | Search model name. |
 
 ```yaml

+ 10 - 5
packages/web/web-search-perplexity/src/provider.ts

@@ -81,6 +81,7 @@ export class PerplexitySearchProvider implements WebSearchProvider {
 
   status(): WebProviderStatus {
     if (this.options.apiKey.length === 0) return { available: false, reason: 'missing-credential' }
+    if (!URL.canParse(this.options.baseURL)) return { available: false, reason: 'misconfigured' }
     return { available: true }
   }
 
@@ -113,11 +114,14 @@ export class PerplexitySearchProvider implements WebSearchProvider {
         const parsed = await response.json() as PerplexityError
         const detail = typeof parsed.error === 'string' ? parsed.error : parsed.error?.message ?? parsed.message
         if (detail !== undefined && detail.length > 0) message = detail
-      } catch {
-        // The HTTP status is already captured in `message` above; a malformed or
-        // non-JSON error body (normal for gateway 5xx/429s) can only cost a
-        // richer provider message, never the real error. `response.json()` is
-        // the sole statement and nothing else of consequence reaches here.
+      } catch (error: unknown) {
+        // An abort fired mid-body must surface as WEB_ABORTED, not be swallowed
+        // into a generic HTTP-error message — cancellation is not a provider
+        // error (the seam's cancellation contract).
+        if (isAbortError(error)) throw new WebError('Perplexity search aborted', 'WEB_ABORTED', { cause: error })
+        // Otherwise: the HTTP status is already captured in `message` above; a
+        // malformed/non-JSON error body (normal for gateway 5xx/429s) can only
+        // cost a richer provider message, never the real error.
       }
       throw new WebError(message, 'WEB_PROVIDER_ERROR')
     }
@@ -126,6 +130,7 @@ export class PerplexitySearchProvider implements WebSearchProvider {
     try {
       payload = await response.json() as PerplexityResponse
     } catch (error: unknown) {
+      if (isAbortError(error)) throw new WebError('Perplexity search aborted', 'WEB_ABORTED', { cause: error })
       throw new WebError(`Perplexity returned an unparseable response body: ${String(error)}`, 'WEB_PROVIDER_ERROR', { cause: error })
     }
     return mapPerplexityResponse(request.query, payload)

+ 19 - 0
packages/web/web-search-perplexity/tests/perplexity.spec.ts

@@ -75,6 +75,11 @@ describe('PerplexitySearchProvider status', () => {
   it('is available with a key', () => {
     expect(new PerplexitySearchProvider(options).status()).toEqual({ available: true })
   })
+
+  it('is misconfigured when the base URL is unparseable', () => {
+    expect(new PerplexitySearchProvider({ ...options, baseURL: 'not a url' }).status())
+      .toEqual({ available: false, reason: 'misconfigured' })
+  })
 })
 
 describe('PerplexitySearchProvider request mapping', () => {
@@ -135,6 +140,20 @@ describe('PerplexitySearchProvider error handling', () => {
       .rejects.toThrow(expect.objectContaining({ code: 'WEB_PROVIDER_ERROR' }))
   })
 
+  it('surfaces an abort during success-body parse as WEB_ABORTED, not provider error', async () => {
+    const body = { json: () => Promise.reject(new DOMException('aborted', 'AbortError')), ok: true, status: 200 }
+    vi.stubGlobal('fetch', vi.fn(async () => body as unknown as Response))
+    await expect(new PerplexitySearchProvider(options).search({ query: 'q' }))
+      .rejects.toThrow(expect.objectContaining({ code: 'WEB_ABORTED' }))
+  })
+
+  it('surfaces an abort during error-body parse as WEB_ABORTED', async () => {
+    const body = { json: () => Promise.reject(new DOMException('aborted', 'AbortError')), ok: false, status: 500 }
+    vi.stubGlobal('fetch', vi.fn(async () => body as unknown as Response))
+    await expect(new PerplexitySearchProvider(options).search({ query: 'q' }))
+      .rejects.toThrow(expect.objectContaining({ code: 'WEB_ABORTED' }))
+  })
+
   it('maps a network failure to WEB_PROVIDER_ERROR', async () => {
     vi.stubGlobal('fetch', vi.fn(() => Promise.reject(new TypeError('connection refused'))))
     await expect(new PerplexitySearchProvider(options).search({ query: 'q' }))