Skip to content

fix: throw the intended error when an app has no similar-apps cluster - #760

Closed
Agi-Asi wants to merge 2 commits into
facundoolano:mainfrom
Agi-Asi:fix/similar-missing-clusters-crash
Closed

Agi-Asi wants to merge 2 commits into
facundoolano:mainfrom
Agi-Asi:fix/similar-missing-clusters-crash

Conversation

@Agi-Asi

@Agi-Asi Agi-Asi commented Aug 27, 2026

Copy link
Copy Markdown

Problem

similar() fails with a TypeError instead of the intended domain error when an app page has no similar-apps cluster (region-restricted apps, apps without recommendations):

Cannot read properties of null (reading 'length')

as reported in #701. Root cause: extractDataWithServiceRequestId returns null/undefined when the ag2B9c service request section is absent from the parsed page, and parseSimilarApps reads .length on that before the intended Error('Similar apps not found') can fire. Callers can't distinguish "this app has no similar apps" from a parser bug.

Fix

  • parseSimilarApps: accept only a non-empty array of clusters; anything else throws the domain error Similar apps not found.
  • processFirstPage (same file): the same guard for the cluster page's app list section — its mapping feeds R.map, which crashes the same way on undefined input.

parseSimilarApps is additionally exported (named export; the default export is unchanged) so the guard can be pinned with a synthetic payload — which live apps happen to lack a cluster changes over time, so a live-repro test would be flaky.

Testing

  • New unit test: a parsed payload without the ag2B9c section throws /Similar apps not found/ instead of a TypeError.
  • Live behavior unchanged for normal apps (com.mojang.minecraftpe, com.spotify.music).
  • npm test: 85 passing, 0 failing; npm run lint: clean

Fixes #701

CI runs 'npm audit' as a required step, and the current lockfile fails
it with 6 vulnerabilities (3 high, 2 moderate, 1 low), all in dev-tool
transitive dependencies:

- serialize-javascript <=7.0.4 (high, RCE + DoS advisories) via mocha
- diff 5.0.0-5.2.1 (jsdiff DoS in parsePatch/applyPatch) via mocha
- js-yaml 4.0.0-4.3.0 (quadratic-CPU DoS advisories) via eslint/mocha
- brace-expansion (DoS family) via minimatch consumers
- ajv <6.14.0 (ReDoS) via eslint

None are fixable by 'npm audit fix' alone: even mocha@latest still pins
vulnerable serialize-javascript/diff ranges. Add npm 'overrides' pinning
each package to its patched line and regenerate the lockfile.

Runtime dependencies are untouched — the diff is dev-tree only, and the
full CI sequence passes clean: npm ci, npm run lint, npm test
(84 passing), npm audit (found 0 vulnerabilities).
similar() crashed with "Cannot read properties of null (reading
'length')" when the app page carried no similar-apps cluster at all
(region-restricted apps, apps without recommendations):
extractDataWithServiceRequestId returns null/undefined when the ag2B9c
service request section is absent, and the guard read .length on it
before the intended Error('Similar apps not found') could fire.

- parseSimilarApps: accept only a non-empty array of clusters; anything
  else throws the domain error, so callers can distinguish "no similar
  apps" from a parser bug.
- processFirstPage: same guard for the cluster page's app list section
  (the mapping feeds R.map, which crashes the same way on undefined).

parseSimilarApps is now also exported for tests so the guard is pinned
with a synthetic payload instead of depending on which live apps happen
to lack a cluster today.

Fixes facundoolano#701
@Agi-Asi

Agi-Asi commented Aug 27, 2026

Copy link
Copy Markdown
Author

Note: CI's npm audit step currently fails on main itself (6 dev-tree vulnerabilities), which cancelled this PR's test matrix. I've rebased this branch on top of the lockfile fix proposed in #762 so the full pipeline (lint, tests on 16/18/20, audit) can run green here. If #762 lands first this PR reduces to its own single commit; happy to rebase either way.

@Agi-Asi
Agi-Asi force-pushed the fix/similar-missing-clusters-crash branch from 12efb52 to 0a4561f Compare August 27, 2026 09:40
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.

Failed to fetch similar apps "Cannot read properties of null (reading 'length')"

2 participants