Skip to content

fix(rsg): don't crash when a Task sets a node-valued field to invalid - #1102

Merged
lvcabral merged 1 commit into
masterfrom
fix/task-set-null-node-field
Jul 29, 2026
Merged

fix(rsg): don't crash when a Task sets a node-valued field to invalid#1102
lvcabral merged 1 commit into
masterfrom
fix/task-set-null-node-field

Conversation

@lvcabral

Copy link
Copy Markdown
Owner

Root cause

Clearing a node-valued field from a task — m.top.payload = invalid — sends a thread update whose value is null (jsValueOf maps invalid to null). The render side reads _address_ off that value to decide whether to reconcile against the node it already holds:

if (!this.inThread && oldValue instanceof Node && oldValue.getAddress() === update.value._address_) {

When the field currently holds a node and the incoming value is null, that throws:

[sg] Error processing task YBPluginRokuVideo (thread 7): Cannot read properties of null (reading '_address_')
TypeError: Cannot read properties of null (reading '_address_')
    at Task.handleSetFieldRequest
    at Task.processThreadUpdate
    at SGRoot.processTasks

The throw escapes processThreadUpdate and aborts the whole task-update pass, so the task's later writes never land and an app waiting on them hangs. Clearing a reference this way is ordinary — analytics and player plugins do it on teardown.

Fix

Guard the address comparison. A null value falls through to brsValueOf, which yields invalid and clears the field as intended.

Pre-existing

Present since the rendezvous thread-update implementation (#856, Feb 2026) — reproduced on clean master, unrelated to the recent SceneGraph work. It is the only unguarded update.value access in the file.

Tests

New task-clear-node-app + a cli.test.js case: a task hands a node to the render thread, then clears it. Asserts the observer sees the node, then invalid, then that the task still reaches completion.

Confirmed the test catches the regression — reverting the one-character guard makes it fail with the original TypeError.

Verification

Clean npm run clean && npm run build (0 TypeScript errors), lint and prettier clean.

Full suite: 2239 passed, 2 failed — both ECP query/r2d2-bitmaps, caused by another process holding port 8060 on the test machine (lsof shows Electron). Unrelated to this change; they pass when that port is free.

🤖 Generated with Claude Code

…invalid

Clearing a node field from a task (`m.top.payload = invalid`) sends an update
whose value is null — jsValueOf maps invalid to null. The render side read
`_address_` off that value to decide whether to reconcile against the node it
already held, so when the field currently held a node the read threw:

    Cannot read properties of null (reading '_address_')
    at Task.handleSetFieldRequest

The throw escapes processThreadUpdate and aborts the whole task-update pass,
so the task's later writes never land and an app waiting on them hangs.

Guard the address comparison. A null value now falls through to brsValueOf,
which yields invalid and clears the field as intended.

Present since the rendezvous thread-update implementation (#856).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@lvcabral
lvcabral merged commit 6af9d4b into master Jul 29, 2026
3 checks passed
@lvcabral
lvcabral deleted the fix/task-set-null-node-field branch July 29, 2026 01:11
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