fix: Harden error handling

[EmandM]

Purpose of this PR

Ensure as many callbacks as possible are wrapped in try/catch blocks to avoid user generated errors throwing into internal netcode functionality.

Jira ticket

Reported internally.

Changelog

• Fixed: Ensures all callbacks are wrapped with exception handling to avoid silent errors.

Documentation

• No documentation changes or additions were necessary.

Testing & QA (How your changes can be verified during release Playtest)

Functional Testing

Manual testing :

• Manual testing done

Automated tests:

• Covered by existing automated tests
• Covered by new automated tests

Does the change require QA team to:

• Review automated tests?
• Execute manual tests?
• Provide feedback about the PR?

If any boxes above are checked the QA team will be automatically added as a PR reviewer.

Up-port

not needed

Backports

Might be worth a backport?

[u-pr]

⚠️ Some settings were ignored in '.pr_agent.toml (repository root)' repo settings

Check the configuration's TOML syntax, permitted settings, and size limit. See the error below.

---

Error message:
Ignored prohibited settings: github_app.handle_pr_actions. Permitted settings were applied.

Configuration content:

# This is the configuration for the u-pr bot
# https://github.cds.internal.unity3d.com/unity/u-pr
# For configuration of this file:
# https://developer.portal.internal.unity.com/catalog/default/component/u-pr/docs/configuration/automatic_runs/

[github_app]
handle_pr_actions = ['opened', 'ready_for_review']  # PR events that auto-run pr_commands
pr_commands       = ["/harness-review"]             # what runs on those events
reviewer_commands = ["/harness-review"]             # what runs when @u-pr is added as reviewer

[u-pr]

💡 Harness Review

This broadly hardens user callback execution, but several exception paths change observable ownership and visibility behavior, and some shared exception boundaries still suppress later lifecycle notifications.

_{Reviewed commit c319570}

_{🤖 Helpful? 👍/👎}

[u-pr]

This helper discards the actual failure reason: every caller now reports SessionOwnerOnly. For example, the locked and request-required branches in ChangeOwnership both reach this method, although the public callback and existing ownership tests distinguish Locked and RequestRequired. Accept the status as a parameter and forward it from each of the five call sites.

_{🤖 Helpful? 👍/👎}

[u-pr]

A configured visibility predicate that throws now grants visibility to that client. NetworkShow, observer setup, connection approval synchronization, and new-client observer updates all consume this return value, so an exception in a predicate intended to hide a private object can add the client as an observer and serialize its spawn/state. Keep the no-handler default as visible, but fail closed (false) when an installed predicate fails.

_{🤖 Helpful? 👍/👎}

[u-pr]

This catch covers three independent lifecycle notifications. On a host, an exception from an OnServerStarted subscriber skips OnClientStarted and OnStarted, even though startup continues successfully after the catch. The same shared-boundary problem occurs in the client/server start and stop paths, as well as where scene generic and specialized callbacks are paired. Invoke each distinct event behind its own exception boundary so one notification cannot suppress the others.

_{🤖 Helpful? 👍/👎}

[u-pr]

This changes the result covered by the existing NetworkManagerStartExceptionTests: it subscribes throwing OnServerStarted/OnClientStarted handlers and asserts that server, host, and client startup return false and tear down the transport. With this catch, these starts now return successfully, but the test was not updated, so that test suite will fail. Update its expectations and cleanup assertions to the new hardening contract (or retain the previous startup-failure behavior).

_{🤖 Helpful? 👍/👎}