fix: add null guards, NaN checks, and try-catch to prevent runtime crashes

[LautaroPetaccio]

Summary

Several code paths in the validator can crash with unhandled exceptions instead of returning proper validation errors. This PR adds defensive guards so the validator fails gracefully with clear error messages rather than throwing.

Why these changes are needed

• entity-structure.ts: The null/empty check on entity.pointers runs after new Set(entity.pointers), which throws a TypeError if pointers is null. The null check is unreachable in the crash case. Reordering puts the null check first so it catches the problem before the Set constructor runs.

• timestamps.ts: All 10 ADR timestamp constants are overridable via environment variables using parseInt(), but none validate the result. If an env var is set to a non-numeric value (e.g. "invalid"), parseInt returns NaN, and since NaN >= anyNumber is always false, the corresponding validateAfterADRxx gate silently disables the wrapped validator. A single misconfigured env var like ADR_45_TIMESTAMP=invalid would disable IPFS hashing, metadata schema, thumbnail, and content validation — with no log message. A shared parseTimestamp helper now falls back to the default when the parsed value is NaN. The incorrect timestamp number in the ADR-74 comment is also fixed.

• scene.ts: metadata?.display.navmapThumbnail uses optional chaining on metadata but not on display. If metadata exists but display is undefined, this throws. Similarly, entity.content.some(...) throws if content is undefined. Added optional chaining on both.

• profile.ts: entity.content.map(...) at two call sites throws if content is undefined. Other locations in the same file already guard with entity.content ?? []; these two were missed. Also, parseUrn() from @dcl/urn-resolver is async and can throw on malformed input, but the wearable and emote URN validators call it without try-catch. If it throws instead of returning null, the entire validator crashes. Added try-catch that treats thrown exceptions the same as unresolvable URNs.

• outfits.ts and utils.ts: Same parseUrn throw risk — wrapped in try-catch.

• subgraph/scenes.ts: The scene access validator accesses estate.owners[0].address and parcel.owners[0].address without checking if the owners array has elements. The subgraph queries filter by createdAt_lte: $timestamp with first: 1, so if no ownership record exists at the queried timestamp, the array is empty and [0].address throws a TypeError. This crashes into the retry() loop (5 retries, 100ms each), then propagates as an internal error. The fix checks owners.length first and returns false (no access) when empty. Also adds isNaN validation after parseInt on pointer coordinates so "abc,def" produces a clear "invalid pointer" error instead of passing NaN to GraphQL queries. The typo "state" in an error message is corrected to "estate".

• on-chain/scenes.ts: Same isNaN guard added for pointer coordinate parsing.

Test plan

• npm run build compiles cleanly
• npm test — all 325 existing tests pass
• npm run lint:check — no errors

🤖 Generated with Claude Code

[github-actions]

Test this pull request

• The package can be tested by running

  yarn upgrade "https://sdk-team-cdn.decentraland.org/@dcl/content-validator/branch/fix/crash-guards-null-safety/dcl-content-validator-7.1.1-24416111545.commit-ef5b5b5.tgz"

[coveralls]

coverage: 83.062% (+1.4%) from 81.689% — fix/crash-guards-null-safety into main

[decentraland-bot]

Review: PR #348 — Null Guards, NaN Checks, and Try-Catch for Runtime Crashes

Verdict: ✅ APPROVE

Solid defensive hardening across the codebase. All changes are correct and prevent real crash scenarios.

Key Changes Reviewed

File	Fix	Assessment
entity-structure.ts	Null check before new Set()	✅ Correct — prevents TypeError on null/undefined pointers
timestamps.ts	parseTimestamp helper with NaN fallback	✅ Correct — prevents silent validator disabling via misconfigured env vars
scene.ts	Optional chaining on display?.navmapThumbnail + content ?? []	✅ Correct
profile.ts	parseUrn try-catch + content ?? [] guards	✅ Correct
outfits.ts / utils.ts	parseUrn try-catch	✅ Correct
subgraph/scenes.ts	Empty owners guard + NaN coord check + "state"→"estate" typo	✅ Correct
on-chain/scenes.ts	NaN coordinate check	✅ Correct

Findings

P2 — Important:

1. [Pattern] The parseUrn try-catch is repeated identically in 4 locations (profile.ts ×2, outfits.ts, utils.ts). Consider extracting a safeParseUrn() utility in utils.ts to centralize this pattern. This would also align with the existing parseUrnNoFail functions found elsewhere in the codebase.

P3 — Nice-to-Have:

2. [Testing] No tests cover: (a) parseTimestamp with NaN input, (b) parseUrn throwing (vs returning null), (c) empty owners array in subgraph scenes. While these are defensive paths, adding at least one test per guard would prevent future regressions.

3. [Testing] In subgraph/scenes.ts, when NaN is detected the code calls controller.abort() and break, but already-queued PQueue tasks may still execute. Consider calling queue.clear() after controller.abort() to prevent unnecessary subgraph queries.

4. [Minor] SCENE_VALIDATIONS_CONCURRENCY parsing (line 55 in subgraph/scenes.ts) could also benefit from the same parseTimestamp-style NaN guard.

CI Status

All checks passing. Coverage increased +0.8%.

Requested by Lautaro Petaccio via Slack

[braianj]

LGTM

[braianj]

maybe this can be only one if, right?