Skip to content

Commit 38822bb

Browse files
wenxi-zengwenxi-zeng
authored
fix event loss in network failure (#64)
* update coroutine and sovran versions * update error reporting logic * update httpclient to report network exception * fix the issue that system status not toggled in a network failure * use semaphore to force synchronization instead of using task.wait
1 parent eca3213 commit 38822bb

File tree

5 files changed

+83
-60
lines changed

5 files changed

+83
-60
lines changed

‎Analytics-CSharp/Analytics-CSharp.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@
3838
<Folder Include="Segment\Analytics\Utilities\" />
3939
</ItemGroup>
4040
<ItemGroup>
41-
<PackageReference Include="Coroutine.NET" Version="1.3.0" />
41+
<PackageReference Include="Coroutine.NET" Version="1.4.0" />
4242
<PackageReference Include="Serialization.NET" Version="1.4.0" />
43-
<PackageReference Include="Sovran.NET" Version="1.3.0" />
43+
<PackageReference Include="Sovran.NET" Version="1.4.0" />
4444
</ItemGroup>
4545
<ItemGroup>
4646
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">

‎Analytics-CSharp/Segment/Analytics/Analytics.cs

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Collections.Generic;
2+
using System.Threading;
23
using global::System;
34
using global::System.Runtime.Serialization;
45
using global::System.Threading.Tasks;
@@ -208,22 +209,31 @@ private void Startup()
208209
Add(new StartupQueue());
209210
Add(new ContextPlugin());
210211

211-
// use Wait() for this coroutine to force completion,
212+
// use semaphore for this coroutine to force completion,
212213
// since Store must be setup before any event call happened.
213-
// Note: Task.Wait() forces internal async methods to run in a synchronized way,
214-
// we should avoid of doing it whenever possible.
214+
SemaphoreSlim semaphore = new SemaphoreSlim(0);
215215
AnalyticsScope.Launch(AnalyticsDispatcher, async () =>
216216
{
217-
// load memory with initial value
218-
_userInfo = UserInfo.DefaultState(Storage);
219-
await Store.Provide(_userInfo);
220-
await Store.Provide(System.DefaultState(Configuration, Storage));
221-
await Storage.Initialize();
222-
}).Wait();
223-
224-
// check settings over the network,
225-
// we don't have to Wait() here, because events are piped in
226-
// StartupQueue until settings is ready
217+
try
218+
{
219+
// load memory with initial value
220+
_userInfo = UserInfo.DefaultState(Storage);
221+
await Store.Provide(_userInfo);
222+
await Store.Provide(System.DefaultState(Configuration, Storage));
223+
await Storage.Initialize();
224+
}
225+
catch (Exception e)
226+
{
227+
ReportInternalError(AnalyticsErrorType.StorageUnknown, e, message: "Unknown Error when restoring settings from storage");
228+
}
229+
finally
230+
{
231+
semaphore.Release();
232+
}
233+
});
234+
semaphore.Wait();
235+
236+
// check settings over the network
227237
AnalyticsScope.Launch(AnalyticsDispatcher, async () =>
228238
{
229239
if (Configuration.AutoAddSegmentDestination)

‎Analytics-CSharp/Segment/Analytics/Errors.cs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public static class ExtensionMethods
4040
public static void ReportInternalError(this Analytics analytics, AnalyticsError error)
4141
{
4242
analytics.Configuration.AnalyticsErrorHandler?.OnExceptionThrown(error);
43-
Analytics.ReportInternalError(error);
43+
Analytics.ReportInternalError(error, error.Message);
4444
}
4545

4646
/// <summary>
@@ -55,12 +55,7 @@ public static void ReportInternalError(this Analytics analytics, AnalyticsErrorT
5555
{
5656
var error = new AnalyticsError(type, message, exception);
5757
analytics.Configuration.AnalyticsErrorHandler?.OnExceptionThrown(error);
58-
Analytics.ReportInternalError(error);
59-
}
60-
61-
public static void ToAnalyticsErrorHandler(this ICoroutineExceptionHandler handler)
62-
{
63-
58+
Analytics.ReportInternalError(error, error.Message);
6459
}
6560
}
6661

‎Analytics-CSharp/Segment/Analytics/Settings.cs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,23 @@ private async Task CheckSettings()
2626
UpdateType updateType = hasSettings ? UpdateType.Refresh : UpdateType.Initial;
2727

2828
await Store.Dispatch<System.ToggleRunningAction, System>(new System.ToggleRunningAction(false));
29-
29+
Settings? settings = null;
3030
await Scope.WithContext(NetworkIODispatcher, async () =>
3131
{
32-
Settings? settings = await httpClient.Settings();
33-
34-
await Scope.WithContext(AnalyticsDispatcher, async () =>
35-
{
36-
if (settings != null)
37-
{
38-
await Store.Dispatch<System.UpdateSettingsAction, System>(new System.UpdateSettingsAction(settings.Value));
39-
Update(settings.Value, updateType);
40-
}
41-
await Store.Dispatch<System.ToggleRunningAction, System>(new System.ToggleRunningAction(true));
42-
});
32+
settings = await httpClient.Settings();
4333
});
34+
35+
if (settings != null)
36+
{
37+
await Store.Dispatch<System.UpdateSettingsAction, System>(new System.UpdateSettingsAction(settings.Value));
38+
}
39+
else
40+
{
41+
settings = systemState._settings;
42+
}
43+
44+
Update(settings.Value, updateType);
45+
await Store.Dispatch<System.ToggleRunningAction, System>(new System.ToggleRunningAction(true));
4446
}
4547
}
4648
}

‎Analytics-CSharp/Segment/Analytics/Utilities/HTTPClient.cs

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -72,49 +72,65 @@ public HTTPClient(string apiKey, string apiHost = null, string cdnHost = null)
7272
public virtual async Task<Settings?> Settings()
7373
{
7474
string settingsURL = SegmentURL(_cdnHost, "/projects/" + _apiKey + "/settings");
75-
Response response = await DoGet(settingsURL);
7675
Settings? result = null;
77-
78-
if (!response.IsSuccessStatusCode)
76+
try
7977
{
80-
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkUnexpectedHttpCode, message: "Error " + response.StatusCode + " getting from settings url");
78+
Response response = await DoGet(settingsURL);
79+
if (!response.IsSuccessStatusCode)
80+
{
81+
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkUnexpectedHttpCode, message: "Error " + response.StatusCode + " getting from settings url");
82+
}
83+
else
84+
{
85+
string json = response.Content;
86+
result = JsonUtility.FromJson<Settings>(json);
87+
}
8188
}
82-
else
89+
catch (Exception e)
8390
{
84-
string json = response.Content;
85-
result = JsonUtility.FromJson<Settings>(json);
91+
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkUnknown, e, "Unknown network error when getting from settings url");
8692
}
93+
8794
return result;
8895
}
8996

9097
public virtual async Task<bool> Upload(byte[] data)
9198
{
9299
string uploadURL = SegmentURL(_apiHost, "/b");
93-
Response response = await DoPost(uploadURL, data);
94-
95-
if (!response.IsSuccessStatusCode)
100+
try
96101
{
97-
Analytics.Logger.Log(LogLevel.Error, message: "Error " + response.StatusCode + " uploading to url");
102+
Response response = await DoPost(uploadURL, data);
98103

99-
switch (response.StatusCode)
104+
if (!response.IsSuccessStatusCode)
100105
{
101-
case var n when n >= 1 && n < 300:
102-
return false;
103-
case var n when n >= 300 && n < 400:
104-
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkUnexpectedHttpCode, message: "Response code: " + n);
105-
return false;
106-
case 429:
107-
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkServerLimited, message: "Response code: 429");
108-
return false;
109-
case var n when n >= 400 && n < 500:
110-
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkServerRejected, message: "Response code: " + n + ". Payloads were rejected by server. Marked for removal.");
111-
return true;
112-
default:
113-
return false;
106+
Analytics.Logger.Log(LogLevel.Error, message: "Error " + response.StatusCode + " uploading to url");
107+
108+
switch (response.StatusCode)
109+
{
110+
case var n when n >= 1 && n < 300:
111+
return false;
112+
case var n when n >= 300 && n < 400:
113+
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkUnexpectedHttpCode, message: "Response code: " + n);
114+
return false;
115+
case 429:
116+
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkServerLimited, message: "Response code: 429");
117+
return false;
118+
case var n when n >= 400 && n < 500:
119+
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkServerRejected, message: "Response code: " + n + ". Payloads were rejected by server. Marked for removal.");
120+
return true;
121+
default:
122+
return false;
123+
}
114124
}
125+
126+
return true;
127+
}
128+
catch (Exception e)
129+
{
130+
AnalyticsRef?.ReportInternalError(AnalyticsErrorType.NetworkUnknown, e, "Unknown network error when uploading to url");
115131
}
116132

117-
return true;
133+
return false;
118134
}
119135

120136
/// <summary>

0 commit comments

Comments
 (0)