RDEV-10097 - Bail out of unbound native object proxy calls

[helderjgoncalves]

Summary

• Adds factory flag BailOutOnUnboundNativeObjectCalls (default true), threaded through ReactView → loader → window flag.
• ViewPropertiesProxy no-ops when the native object/method is missing, and swallows CefGlue "was not found" / bind failures from teardown/remount races; other errors still throw. Flag off restores previous behaviour.
• Patch bump to 5.120.7.

Test plan

• Open ODC Studio with this ReactView build (or Phoenix wired to 5.120.7 + FT on): switch modules / close editors while hovering trees — console should no longer flood with $editorCanvas$MultipleEditors was not found or proxy.<computed> TypeErrors for mouseEnterNode / mouseLeaveTree
• With FT off, previous error behaviour returns
• Smoke: normal tree selection, MultipleEditors breadcrumb refresh, and editor open still work when views are alive

Made with Cursor

[helderjgoncalves]

Review round addressed in 5ed8d66.

1. The flag-off path is byte-identical to the previous code again. invokeNative ends on nativeObject[key].apply(window, args), so with BailOutOnUnboundNativeObjectCalls off a call to an object that is no longer bound still throws naming the method (Cannot read properties of undefined (reading 'mouseLeaveTree')) rather than losing it to a .apply on undefined. The method name is what identifies the caller, so it was worth keeping. All new behaviour is now behind the flag.

2. view.isReleased is the primary signal. createPropertiesProxy takes the ViewMetadata (the call site in Loader.ts already had it in scope) and short-circuits before the binding round-trip, using the same idiom as the other three readers of isReleased. The rejection-text match stays as the backstop for the narrower case where the object is still on the window while the host has already unregistered it.

3. "Failed to create native object" is no longer swallowed. It is raised by HandleNativeObjectRegistration when the V8 context cannot be entered at registration time, which is a real failure and not a teardown race, so it propagates as before. The predicate is down to the genuine unbound-object rejection only.

4. Dropped calls are logged. console.debug naming the object and method, on every bail-out path. Bailing out silently makes a native object that was never registered indistinguishable from one that went away with its view, and only one of those is a bug.

5. Tests added in Tests.ReactView/UnboundNativeObjectCallsTests.cs: the unbound object is ignored, the destroyed view's object is ignored (via the isReleased path after hideInnerView), and with the flag off the call fails with a message naming methodCalled. The unbound state is reproduced deterministically rather than by racing a teardown - the test app deletes the inner view's global and verifies it is gone first, which is the exact production mechanism, since DeleteNativeObject removes the global but leaves the already-completed entry in _pendingBoundQueryTasks, so checkObjectBound resolves straight away and bindNativeObject hands back undefined. Each test also hooks InnerView.MethodCalled, so a call that did get through fails the assertion instead of passing silently.

---

Caveat worth knowing about isReleased: it is only ever assigned inside releaseView, which onChildViewRemoved calls under the EnsureViewPluginsAreDisposed flag. With that flag off, the fast path simply never fires and the structural check plus the error text carry the whole thing - correctness does not depend on it. Every existing reader of isReleased has the same characteristic, so this adds no new coupling, and I deliberately left onChildViewRemoved alone rather than changing behaviour under the other toggle.

The new tests could not be run locally. The whole Tests.ReactView suite aborts on macOS with InvalidOperationException: Call from invalid thread out of AppBuilder.SetupWithoutStarting, because Avalonia's native backend needs the process main thread while TestBase spins its own UI thread. This is pre-existing and not specific to the new tests - PropertiesValuesTests fails identically. They need CI or Windows to actually execute. What was verified locally: the loader type-checks clean, the compiled Loader.js carries the change, and Tests.ReactView builds with 0 errors on both x64 and ARM64.

Version stays at 5.120.7 - same unpublished slot.

[helderjgoncalves]

Narrowed the bail out to calls that return nothing (a74a8ec).

Why

Stress testing this in ODC Studio, by switching views as fast as possible with Verbose logging on, showed the dropped calls split into two very different classes.

Void notifications — dropping these is correct, and it is what the original report was about:

Ignored call to "$rightPane.treeAggregator.Interfaces$TreeView.mouseLeaveTree" the object is no longer bound
Ignored call to "$rightPane.treeAggregator.Interfaces$TreeView.mouseEnterNode" the object is no longer bound
Ignored call to "$rightPane.treeAggregator.Interfaces$TreeView.doubleClickNode" the object is no longer bound

Value returning queries — dropping these resolves the promise with undefined, and the component that is still mounted writes that into its state:

Ignored call to "$topPane.toolbar$ToolbarView.getToolbarItems" Object named ... was not found. Make sure it was registered before.
Ignored call to "$topPane$TopPaneView.getTopPaneInfo" (same)
Ignored call to "$leftPane$LeftPaneView.getLeftPaneState" (same)
Ignored call to "$statusBar$StatusBarView.getSpaceInfo" (same)
Ignored call to "$editorCanvas.canvas-8$UIEditorView.getGridLayout" the view was destroyed

Each of those came back as a crash further away from its cause, in the consuming app: TypeError: Cannot read properties of undefined (reading 'tools') after await this.props.getTools(...), Cannot destructure property 'name' of 'this.state.objectsInfo' as it is undefined after await this.props.getObjectsInfo(), and the same shape in PropertiesList.view.tsx, TreeAggregator.tsx and List.tsx. Resolving with undefined is not safe even for an unmounted component: some callers dereference the result before any setState.

What changed

The host now reflects over the native object it registered, matches its methods against the module's events, and sends the loader the names of the ones that return nothing, as one more loadComponent argument. createPropertiesProxy only drops a call when the method is in that set. A method that returns a value, or one that could not be matched (so nothing is known about it), falls through to the code path that existed before this branch and fails exactly as it did, naming the method it was trying to reach. Bailing out only for known void methods is the safe default: unknown metadata costs the bail out, never the correctness of a call.

A method returning plain Task counts as void, since the promise the caller awaits carries no value either way; Task<T> does not.

UnboundNativeObjectCallsTests now covers both classes, for the unbound object and for the destroyed view: the void call is ignored, the value returning call still fails.

[helderjgoncalves]

Re-scoped: the axis is released vs live, not void vs value

Runtime evidence and a product decision moved this branch off the void / value distinction that a74a8ec introduced. Pushed as 9a85e09, a deliberate narrowing rather than a revert.

Why the void / value axis was wrong

The priority is that a render crash is acceptable but silently breaking view-to-presenter communication is not. Measured against that, return type is the wrong thing to key on: dropping a void call on a view that is still alive discards a real user interaction. A doubleClickNode or a mouseEnterNode that vanishes leaves a tree that looks alive and does nothing, with nothing anywhere reporting it. That is worse than the crash it was trying to avoid, because it is invisible.

What actually separates the two cases

View genuinely released (view.isReleased) — there is no native object left to reach and nobody left to receive a result, so the call is dropped and logged, whatever its return type. This is the case observed as Ignored call to "$editorCanvas.canvas-8$UIEditorView.getGridInfo" the view was destroyed, which caused no damage.

View live but its native object unbound — a broken channel to the presenter, never silently swallowed, whatever its return type. These go back to surfacing exactly as they did before the branch existed:

• $topPane.toolbar$ToolbarView.getToolbarItems and $leftPane$LeftPaneView.getLeftPaneState rejecting with Object named ... was not found. Make sure it was registered before.
• the flood of $rightPane.treeAggregator.Interfaces$TreeView.mouseEnterNode ... the object is no longer bound

What that removed

Because isReleased is checked before the call is dispatched, a released view never reaches native — so every rejection the catch used to swallow necessarily came from a live view, which is precisely the case that must stay loud. The catch and isUnboundNativeObjectError therefore go, and with them the post-bind unbound-object check and the whole reflected void-member metadata mechanism from a74a8ec (GetVoidNativeObjectMethods / ReturnsNothing, the extra loadComponent argument, the voidNativeObjectMethods loader parameter, and the RegisterNativeObject signature change that existed only to feed it).

The native invocation in ViewPropertiesProxy is now byte-identical to its pre-branch form, so a live view's failure produces the original TypeError naming the method it was trying to reach. BailOutOnUnboundNativeObjectCalls stays with its full plumbing, now gating only the dropped call into a destroyed view; its doc comment says exactly that.

Net effect on the diff against master: the production change is down to the toggle plumbing plus a 13-line isReleased guard.

Not in scope, and what comes next

The host-lifecycle bug that strands the native objects of live views is the real defect behind the getToolbarItems / mouseEnterNode reports. It is being diagnosed separately and is deliberately not addressed here — this branch only stops the loader from hiding it.

Once that diagnosis lands, the planned follow-up is a bounded wait for a rebind, so that an in-flight call survives a legitimate re-registration instead of failing on a race, without ever silently dropping a call from a live view.

Verification

• Loader type-checks clean with the pinned TypeScript 5.4.3 (--noEmit against ReactViewResources/Loader/tsconfig.json).
• dotnet build Tests.ReactView/Tests.ReactView.csproj /p:Platform=ARM64 succeeds, 0 errors.
• UnboundNativeObjectCallsTests reworked to the new contract: destroyed view ignored for both void and value-returning methods; live view with an unbound object fails for both, with the error naming the method; and with the toggle off, the destroyed-view call fails too. The suite cannot be executed on macOS (InvalidOperationException: Call from invalid thread from AppBuilder.SetupWithoutStarting, pre-existing for every test in the project), so it is verified to compile only.