Catch JsonConvert_DeserializeObject exceptions too

[FrozenAlex]

Kinda missed that, should prevent the game from crashing when parsing invalid json for v4 maps and other things that use deserialize object

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling and logging for multiplayer status data synchronization, enhancing stability when processing remote player information.

[coderabbitai]

Walkthrough

Added exception handling to JSON deserialization in the MultiplayerStatusModel hook. The hook now wraps the deserialization call in a try/catch block that logs a warning with backtrace on failure and returns nullptr, improving robustness against malformed data.

Changes

Cohort / File(s)	Summary
Exception Handling in JSON Deserialization src/Hooks/MultiplayerStatusModelHooks.cpp	Added try/catch wrapper around JsonConvert_DeserializeObject_MultiplayerStatusData to handle exceptions during deserialization, log warnings with backtrace, and return nullptr on failure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• Fix crashes on any invalid json parsed by unity and bump dependencies #40: Adds identical try/catch exception handling to the same JSON deserialization hook with consistent error logging and nullptr return behavior.

Poem

🐰 Hop hop, let the errors flow,
We catch them before they steal the show,
With logged traces and nullptr's care,
No bad JSON will find despair! 📋✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Catch JsonConvert_DeserializeObject exceptions too' directly describes the main change: adding exception handling to the JsonConvert_DeserializeObject call in the hook to prevent crashes from invalid JSON deserialization.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Hooks/MultiplayerStatusModelHooks.cpp (1)

61-61: ⚠️ Potential issue | 🟠 Major

Same issue: custom type path missing exception handling.

For consistency with the fix on line 28, this MpStatusData::New_ctor(value) call should also be wrapped in try/catch.

🐛 Proposed fix

     if (method_inst && method_inst->type_argc == 1 && il2cpp_functions::type_equals(method_inst->type_argv[0], il2cpp_utils::il2cpp_type_check::il2cpp_no_arg_type<GlobalNamespace::MultiplayerStatusData*>::get())) {
         // THIS IS OUR CALL! LETS DO OUR CUSTOM BEHAVIOR HERE
         DEBUG("JsonUtility_FromJson_MultiplayerStatusData CUSTOM TYPE RETURN");
-        return MultiplayerCore::Models::MpStatusData::New_ctor(value);
+        try {
+            return MultiplayerCore::Models::MpStatusData::New_ctor(value);
+        } catch (...) {
+            WARNING("JsonUtility_FromJson_MultiplayerStatusData exception caught in custom type path, we have invalid json somewhere, printing backtrace for debugging...");
+            Paper::Logger::Backtrace(40);
+            return nullptr;
+        }
     } else {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Hooks/MultiplayerStatusModelHooks.cpp` at line 61, Wrap the call to
MultiplayerCore::Models::MpStatusData::New_ctor(value) in a try/catch like the
earlier fix: catch the exception (same exception type used at the other fix),
log the caught exception with the same logger/message pattern used on line 28,
and return the same error sentinel (e.g., nullptr or the same failure return) so
behavior is consistent with the prior change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Hooks/MultiplayerStatusModelHooks.cpp`:
- Line 28: Wrap the call to
MultiplayerCore::Models::MpStatusData::New_ctor(value) in a try/catch to handle
deserialization exceptions (e.g., catch std::exception and any
framework-specific exceptions) so invalid JSON doesn't propagate; on catch, log
the error with context (including the exception.what() or message) and return a
safe fallback (e.g., nullptr or an empty optional) instead of allowing the
exception to escape from the function in MultiplayerStatusModelHooks.cpp where
the return currently is MultiplayerCore::Models::MpStatusData::New_ctor(value).

---

Outside diff comments:
In `@src/Hooks/MultiplayerStatusModelHooks.cpp`:
- Line 61: Wrap the call to
MultiplayerCore::Models::MpStatusData::New_ctor(value) in a try/catch like the
earlier fix: catch the exception (same exception type used at the other fix),
log the caught exception with the same logger/message pattern used on line 28,
and return the same error sentinel (e.g., nullptr or the same failure return) so
behavior is consistent with the prior change.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5198fb82-5938-47b3-83ea-ad3db8574cfd

📥 Commits

Reviewing files that changed from the base of the PR and between ad4006e and 9bac29d.

📒 Files selected for processing (1)

• src/Hooks/MultiplayerStatusModelHooks.cpp