Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLAT-11641 handle json serialisation errors and remove payloads #773

Merged
merged 11 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## TBD ()

### Enhancements

- Improved handling of serialisation errors when flushing the event cache [#773](https://github.com/bugsnag/bugsnag-unity/pull/773)

## 7.7.1 (2024-01-25)

### Bug Fixes
Expand Down
12 changes: 12 additions & 0 deletions features/csharp/csharp_persistence.feature
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,15 @@ Feature: Unity Persistence
And the exception "message" equals "PersistDeviceId"
And the error payload field "events.0.device.id" equals the stored value "device_id"

Scenario: Handle Corrupt Json
And I run the game in the "CorruptedCacheFile" state
And I wait for requests to persist
And I close the Unity app
And On Mobile I relaunch the app
And I run the game in the "PersistEventReport" state
And I wait for requests to persist
And I wait to receive 1 errors
And the error is valid for the error reporting API sent by the Unity notifier
And the exception "message" equals "Error 2"


18 changes: 17 additions & 1 deletion features/fixtures/maze_runner/Assets/Scenes/MainScene.unity
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ RenderSettings:
m_ReflectionIntensity: 1
m_CustomReflection: {fileID: 0}
m_Sun: {fileID: 0}
m_IndirectSpecularColor: {r: 0.3731192, g: 0.38073966, b: 0.35872677, a: 1}
m_IndirectSpecularColor: {r: 0.3731316, g: 0.38074902, b: 0.3587254, a: 1}
m_UseRadianceAmbientProbe: 0
--- !u!157 &3
LightmapSettings:
Expand Down Expand Up @@ -1537,6 +1537,7 @@ GameObject:
- component: {fileID: 1523264808}
- component: {fileID: 1523264809}
- component: {fileID: 1523264810}
- component: {fileID: 1523264811}
m_Layer: 0
m_Name: Persistence
m_TagString: Untagged
Expand Down Expand Up @@ -1678,6 +1679,21 @@ MonoBehaviour:
CustomStacktrace: 'Main.CUSTOM1 () (at Assets/Scripts/Main.cs:123)

Main.CUSTOM2 () (at Assets/Scripts/Main.cs:123)'
--- !u!114 &1523264811
MonoBehaviour:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
m_GameObject: {fileID: 1523264801}
m_Enabled: 1
m_EditorHideFlags: 0
m_Script: {fileID: 11500000, guid: 395039af028ee4010baf61afa7f7e066, type: 3}
m_Name:
m_EditorClassIdentifier:
CustomStacktrace: 'Main.CUSTOM1 () (at Assets/Scripts/Main.cs:123)

Main.CUSTOM2 () (at Assets/Scripts/Main.cs:123)'
--- !u!1 &1746155638
GameObject:
m_ObjectHideFlags: 0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using System.Collections;
using System.Collections.Generic;
using System.IO;
using UnityEngine;

public class CorruptedCacheFile : Scenario
{

public override void PrepareConfig(string apiKey, string host)
{
base.PrepareConfig(apiKey, host);
if (Application.platform == RuntimePlatform.IPhonePlayer)
{
Configuration.EnabledErrorTypes.OOMs = false;
}
}

public override void Run()
{
var dirPath = Application.persistentDataPath + "/Bugsnag/Events";
if (!Directory.Exists(dirPath))
{
Directory.CreateDirectory(dirPath);
}
File.WriteAllText(dirPath + "/f04274f7-6f7d-448e-b62e-486cc019a708.event", "NOT JSON");
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 34 additions & 4 deletions src/BugsnagUnity/Delivery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -524,21 +524,51 @@ private IEnumerator ProcessCachedItems(Type t)

try
{
// if something goes wrong at this stage then we silently discard the file as it's most likely that the file wasn't fully serialised to disk
payloadDictionary = ((JsonObject)SimpleJson.DeserializeObject(payloadJson)).GetDictionary();
}
catch (Exception e)
catch
{
Debug.LogException(new Exception("Bugsnag Error. Deserialising a cached payload for delivery failed: " + e.Message + " Cached json: " + payloadJson));
if (isSession)
{
_cacheManager.RemoveCachedSession(id);
}
else
{
_cacheManager.RemoveCachedEvent(id);
}
continue;
}

if (isSession)
{
Deliver(new SessionReport(_configuration, payloadDictionary));
SessionReport sessionReport = null;
try
{
sessionReport = new SessionReport(_configuration, payloadDictionary);
}
catch
{
// this will be internally reported in a future update
_cacheManager.RemoveCachedSession(id);
continue;
}
Deliver(sessionReport);
}
else
{
Deliver(new Report(_configuration, payloadDictionary));
Report report = null;
try
{
report = new Report(_configuration, payloadDictionary);
}
catch
{
// this will be internally reported in a future update
_cacheManager.RemoveCachedEvent(id);
continue;
}
Deliver(report);
}

yield return new WaitUntil(() => CachedPayloadProcessed(id));
Expand Down
6 changes: 5 additions & 1 deletion src/BugsnagUnity/Native/Fallback/CacheManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,13 @@ private void DeleteFile(string path)

private void WriteFile(string path, string data)
{
// not using File.WriteAllText to avoid under the hood buffering. We want the file to be written immediately to avoid truncation in case of a crash
try
{
File.WriteAllText(path, data);
var file = File.Create(path);
file.Write(System.Text.Encoding.UTF8.GetBytes(data), 0, data.Length);
file.Flush();
file.Close();
}catch { }
}

Expand Down