Skip to content

fix(gateway): guard non-object video upstream JSON#2785

Merged
steebchen merged 1 commit into
mainfrom
cancun-v3
Jun 22, 2026
Merged

fix(gateway): guard non-object video upstream JSON#2785
steebchen merged 1 commit into
mainfrom
cancun-v3

Conversation

@steebchen

@steebchen steebchen commented Jun 22, 2026

Copy link
Copy Markdown
Member

Problem

Production gateway crashed with:

TypeError: Cannot read properties of null (reading 'msg')
    at fetchUpstreamJson (/app/src/videos/videos.ts:2728:15)
    at uploadAtlasCloudMedia (.../videos.ts:3270)
    at getAtlasCloudImageUrl (.../videos.ts:3360)
    at createAtlasCloudVideoJob (.../videos.ts:3390)

fetchUpstreamJson annotated the parsed response body as Record<string, unknown>, but JSON.parse returns any. When an upstream provider responded with a body of the literal text null (or any non-object JSON primitive), body became null, and the subsequent typeof body.msg dereferenced null and threw.

Fix

Guard the parse result: only treat a non-null object as the body. Anything else (null / primitive) falls back to wrapping the raw text in an error object — the same path already used for non-JSON responses.

Testing

  • pnpm turbo run build --filter=gateway passes
  • Formatted with prettier

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling for upstream API responses to more reliably process and report malformed or unexpected data formats.

fetchUpstreamJson typed the parsed body as Record<string, unknown> but
JSON.parse returns any, so an upstream response body of the literal
`null` (or any JSON primitive) became null and crashed on `body.msg`
with "Cannot read properties of null (reading 'msg')". Guard the parse
result to only treat non-null objects as the body, otherwise fall back
to wrapping the raw text as an error.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@steebchen steebchen enabled auto-merge June 22, 2026 00:57
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

In fetchUpstreamJson, the JSON parse result is now typed as unknown and conditionally narrowed to Record<string, unknown> only when the value is a non-null object. Non-object responses are replaced with { error: { message: text } } before the result is used for error detection and logging.

Changes

Defensive Upstream JSON Parsing

Layer / File(s) Summary
Upstream JSON parse guard
apps/gateway/src/videos/videos.ts
fetchUpstreamJson parses response JSON into unknown, uses the result as a Record<string, unknown> only when it is a non-null object, and substitutes { error: { message: text } } for any non-object value.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(gateway): guard non-object video upstream JSON' directly summarizes the main change - adding a guard clause to handle non-object JSON responses in the gateway's video upstream handling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cancun-v3

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1ea677daa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2719 to +2721
typeof parsed === "object" && parsed !== null
? (parsed as Record<string, unknown>)
: {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve JSON string upload URLs

When AtlasCloud's media upload responds with a valid JSON string containing a URL, such as "https://...", this branch now wraps the parsed string in { error: { message: text } } instead of returning the string. uploadAtlasCloudMedia immediately passes this value to extractAtlasCloudUploadedMediaUrl, which has an explicit top-level string URL case, so those uploads regress from succeeding to AtlasCloud media upload did not return a usable URL; the null crash guard should not discard string primitives that existing extraction logic can handle.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/gateway/src/videos/videos.ts`:
- Around line 2719-2721: The object type guard in the parsed-body condition is
accepting arrays because typeof array returns "object" in JavaScript, which
causes array payloads to bypass the fallback handler and be returned as
Record<string, unknown>, violating the intended response contract. Modify the
guard expression that checks `typeof parsed === "object" && parsed !== null` to
additionally exclude arrays by adding an Array.isArray check, ensuring only
plain objects pass through while arrays fall through to the fallback case.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d273d871-b5d3-4b38-9dd2-a76934ff05aa

📥 Commits

Reviewing files that changed from the base of the PR and between 6a55a0a and d1ea677.

📒 Files selected for processing (1)
  • apps/gateway/src/videos/videos.ts

Comment on lines +2719 to +2721
typeof parsed === "object" && parsed !== null
? (parsed as Record<string, unknown>)
: {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Exclude arrays in the parsed-body guard to preserve response contract.

On Line 2719, the object check still accepts arrays (typeof [] === "object"), so array JSON payloads bypass the fallback and are returned as Record<string, unknown>. That contradicts the intended behavior for non-object payloads and can leak invalid body shapes downstream.

Suggested fix
-			body =
-				typeof parsed === "object" && parsed !== null
+			body =
+				typeof parsed === "object" &&
+				parsed !== null &&
+				!Array.isArray(parsed)
 					? (parsed as Record<string, unknown>)
 					: {
 							error: {
 								message: text,
 							},
 						};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
typeof parsed === "object" && parsed !== null
? (parsed as Record<string, unknown>)
: {
body =
typeof parsed === "object" &&
parsed !== null &&
!Array.isArray(parsed)
? (parsed as Record<string, unknown>)
: {
error: {
message: text,
},
};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/gateway/src/videos/videos.ts` around lines 2719 - 2721, The object type
guard in the parsed-body condition is accepting arrays because typeof array
returns "object" in JavaScript, which causes array payloads to bypass the
fallback handler and be returned as Record<string, unknown>, violating the
intended response contract. Modify the guard expression that checks `typeof
parsed === "object" && parsed !== null` to additionally exclude arrays by adding
an Array.isArray check, ensuring only plain objects pass through while arrays
fall through to the fallback case.

@steebchen steebchen added this pull request to the merge queue Jun 22, 2026
Merged via the queue into main with commit 9af0d96 Jun 22, 2026
17 checks passed
@steebchen steebchen deleted the cancun-v3 branch June 22, 2026 01:15
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