Skip to content

fix(brave-search): tolerate malformed result entries - #2998

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/brave-search-skip-malformed-results
Open

fix(brave-search): tolerate malformed result entries#2998
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/brave-search-skip-malformed-results

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Why this matters

Brave Search is a live external I/O boundary used directly and through Perplexica's SearXNG compatibility path. One null, scalar, or non-string field inside an otherwise valid upstream results array currently throws during .trim(), turning the whole search into a 500 and discarding valid sibling results.

Root cause and invariant

The proxy checked that web.results was an array but assumed every element was an object and every mapped field was a string. The invariant is that malformed individual upstream records cannot crash the service; only object records with a usable string URL become public results, while valid siblings retain their stable shapes.

The shared extractor now rejects non-object entries and a single string normalizer handles title, URL, and description in both response formats.

Overlap check

Searched open and closed PRs for brave search malformed upstream result, invalid upstream payload, and the changed files. Open same-production-file PR #2734 adds caller authentication to the native route and does not touch upstream result mapping. No semantic duplicate was found.

Regression coverage

The existing loopback end-to-end harness now makes the stub Brave server return null, scalar, wrong-typed fields, and one valid sibling. It exercises both real proxy child processes and asserts /v1/search plus SearXNG /search return 200 with exactly the valid normalized result.

Validation

  • bash ods/tests/test-brave-search-searxng-compat.sh ? all checks passed
  • node --check ods/extensions/services/brave-search/proxy.mjs
  • node --check ods/tests/test-brave-search-searxng-compat.mjs
  • git diff --check

Tradeoffs and rollback

Malformed entries are skipped rather than failing the whole response, matching the existing behavior for empty URLs and malformed top-level shapes. ODS does not invent values for invalid fields. Revert restores the per-item crash behavior; no configuration or data migration is required.

@tang-vu

tang-vu commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Batch compatibility receipt (2026-08-23)

Validated merge order: #2989 ? #2990 ? #2993 ? #2991 ? #2992 ? #2994 ? #2995 ? #2996 ? #2997 ? #2998. The changes are independently useful; this order only reconciles shared model-router and magic-link files.

Synthetic integration head: origin/batch/quality-ten-20260822-round2-integration at 91eb730d. The only textual conflict was the two model-router tests inserting at the same class boundary; the integration resolution retains both contracts. Magic-link changes merged cleanly.

Combined validation on that exact head:

  • remote-provider egress contract: 16 passed
  • Token Spy suites: 22 passed, 1 skipped (live PostgreSQL availability)
  • model-router suite: 55 passed
  • APE suite: 29 passed
  • dashboard OAuth + magic-link suites: 118 passed, 2 platform skips
  • Brave Search loopback E2E: all checks passed
  • git diff --check: passed

All required GitHub checks are green across the batch. #2992 initially hit a transient openSUSE repository/mirror failure while installing rsync; a clearly labeled empty retry commit reran the unchanged tree, and openSUSE plus the full matrix passed.

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.

1 participant