Skip to content
This repository was archived by the owner on Apr 24, 2024. It is now read-only.

Commit 1cd8582

Browse files
authored
Merge pull request #164 from North-Two-Five/issue#163-Events-not-sent-to-Segment-servers-Unexpected-Status-Code
Issue#163 events not sent to segment servers unexpected status code
2 parents 230fed0 + 6acf9a3 commit 1cd8582

File tree

8 files changed

+341
-8
lines changed

8 files changed

+341
-8
lines changed

Analytics/Client.cs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,23 @@ internal Client(string writeKey, Config config, IRequestHandler requestHandler)
6161

6262
if (requestHandler == null)
6363
{
64-
if (config.Send)
65-
requestHandler = new BlockingRequestHandler(this, config.Timeout);
66-
else
64+
if (config.Send)
65+
{
66+
if (config.MaxRetryTime.HasValue)
67+
{
68+
requestHandler = new BlockingRequestHandler(this, config.Timeout, new Backo(max: (Convert.ToInt32(config.MaxRetryTime.Value.TotalSeconds) * 1000), jitter: 5000));
69+
}
70+
else
71+
{
72+
requestHandler = new BlockingRequestHandler(this, config.Timeout);
73+
}
74+
75+
}
76+
else
77+
{
6778
requestHandler = new FakeRequestHandler(this);
79+
}
80+
6881
}
6982

7083
IBatchFactory batchFactory = new SimpleBatchFactory(this._writeKey);

Analytics/Config.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public class Config
3030

3131
internal TimeSpan Timeout { get; set; }
3232

33+
internal TimeSpan? MaxRetryTime { get; set; }
34+
3335
internal int FlushIntervalInMillis { get; private set; }
3436

3537
public bool Send { get; set; }
@@ -50,6 +52,7 @@ public class Config
5052
/// <param name="gzip">Compress data w/ gzip before dispatch</param>
5153
/// <param name="send">Send data to Segment</param>
5254
/// <param name="userAgent">Sets User Agent Header</param>
55+
/// <param name="maxRetryTime">Max Amount of time to retry request when server timeout occurs</param>
5356
public Config(
5457
string host = "https://api.segment.io",
5558
string proxy = null,
@@ -61,7 +64,8 @@ public Config(
6164
double flushInterval = 10,
6265
bool gzip = false,
6366
bool send = true,
64-
string userAgent = null
67+
string userAgent = null,
68+
TimeSpan? maxRetryTime = null
6569
)
6670
{
6771
this.Host = host;
@@ -75,6 +79,7 @@ public Config(
7579
this.Send = send;
7680
this.UserAgent = userAgent ?? GetDefaultUserAgent();
7781
this.Threads = threads;
82+
this.MaxRetryTime = maxRetryTime;
7883
}
7984

8085
private static string GetDefaultUserAgent()
@@ -117,6 +122,17 @@ public Config SetTimeout(TimeSpan timeout)
117122
return this;
118123
}
119124

125+
/// <summary>
126+
/// Sets the maximum amount of retry time for request to flush to the server when Timeout or error occurs.
127+
/// </summary>
128+
/// <param name="timeout"></param>
129+
/// <returns></returns>
130+
public Config SetMaxRetryTime(TimeSpan maxRetryTime)
131+
{
132+
this.MaxRetryTime = maxRetryTime;
133+
return this;
134+
}
135+
120136
/// <summary>
121137
/// Sets the maximum amount of items that can be in the queue before no more are accepted.
122138
/// </summary>

Analytics/Request/BlockingRequestHandler.cs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ internal class BlockingRequestHandler : IRequestHandler
9797
internal BlockingRequestHandler(Client client, TimeSpan timeout) : this(client, timeout, null, new Backo(max: 10000, jitter: 5000)) // Set maximum waiting limit to 10s and jitter to 5s
9898
{
9999
}
100+
internal BlockingRequestHandler(Client client, TimeSpan timeout, Backo backo) : this(client, timeout, null, backo)
101+
{
102+
}
100103
#if NET35
101104

102105
internal BlockingRequestHandler(Client client, TimeSpan timeout, IHttpClient httpClient, Backo backo)
@@ -272,12 +275,24 @@ public async Task MakeRequest(Batch batch)
272275
{
273276
response = await _httpClient.PostAsync(uri, content).ConfigureAwait(false);
274277
}
275-
catch (TaskCanceledException)
278+
catch (TaskCanceledException e)
276279
{
280+
Logger.Info("HTTP Post failed with exception of type TaskCanceledException", new Dict
281+
{
282+
{ "batch id", batch.MessageId },
283+
{ "reason", e.Message },
284+
{ "duration (ms)", watch.ElapsedMilliseconds }
285+
});
277286
retry = true;
278287
}
279-
catch (HttpRequestException)
288+
catch (HttpRequestException e)
280289
{
290+
Logger.Info("HTTP Post failed with exception of type HttpRequestException", new Dict
291+
{
292+
{ "batch id", batch.MessageId },
293+
{ "reason", e.Message },
294+
{ "duration (ms)", watch.ElapsedMilliseconds }
295+
});
281296
retry = true;
282297
}
283298

@@ -315,10 +330,11 @@ public async Task MakeRequest(Batch batch)
315330
{ "duration (ms)", watch.ElapsedMilliseconds }
316331
});
317332
}
318-
continue;
319333
}
320334
else
321335
{
336+
//HTTP status codes smaller than 500 or greater than 600 except for 429 are either Client errors or a correct status
337+
//This means it should not retry
322338
break;
323339
}
324340
}

Test.Net35/ConnectionTests.cs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,32 @@ public void RetryErrorTestNet35()
4949
{
5050
Stopwatch watch = new Stopwatch();
5151

52+
// Set invalid host address and make timeout to 1s
53+
var config = new Config().SetAsync(false);
54+
config.SetHost("https://fake.segment-server.com");
55+
config.SetTimeout(new TimeSpan(0, 0, 1));
56+
config.SetMaxRetryTime(new TimeSpan(0, 0, 10));
57+
Analytics.Initialize(Constants.WRITE_KEY, config);
58+
59+
// Calculate working time for Identiy message with invalid host address
60+
watch.Start();
61+
Actions.Identify(Analytics.Client);
62+
watch.Stop();
63+
64+
Assert.AreEqual(1, Analytics.Client.Statistics.Submitted);
65+
Assert.AreEqual(0, Analytics.Client.Statistics.Succeeded);
66+
Assert.AreEqual(1, Analytics.Client.Statistics.Failed);
67+
68+
// Handling Identify message will take more than 10s even though the timeout is 1s.
69+
// That's because it retries submit when it's failed.
70+
Assert.AreEqual(true, watch.ElapsedMilliseconds > 10000);
71+
}
72+
73+
[Test()]
74+
public void RetryErrorWithDefaultMaxRetryTimeTestNet35()
75+
{
76+
Stopwatch watch = new Stopwatch();
77+
5278
// Set invalid host address and make timeout to 1s
5379
var config = new Config().SetAsync(false);
5480
config.SetHost("https://fake.segment-server.com");
@@ -83,6 +109,7 @@ public void RetryServerErrorTestNet35()
83109
var config = new Config().SetAsync(false);
84110
config.SetHost(DummyServerUrl);
85111
config.SetTimeout(new TimeSpan(0, 0, 1));
112+
config.SetMaxRetryTime(new TimeSpan(0, 0, 10));
86113
Analytics.Initialize(Constants.WRITE_KEY, config);
87114

88115
var TestCases = new RetryErrorTestCase[]
@@ -149,7 +176,85 @@ public void RetryServerErrorTestNet35()
149176
}
150177
}
151178

179+
[Test()]
180+
public void RetryServerErrorWithDefaultMaxRetryTimeTestNet35()
181+
{
182+
Stopwatch watch = new Stopwatch();
152183

184+
string DummyServerUrl = "http://localhost:9696";
185+
using (var DummyServer = new WebServer(DummyServerUrl))
186+
{
187+
DummyServer.RunAsync();
188+
189+
// Set invalid host address and make timeout to 1s
190+
var config = new Config().SetAsync(false);
191+
config.SetHost(DummyServerUrl);
192+
config.SetTimeout(new TimeSpan(0, 0, 1));
193+
Analytics.Initialize(Constants.WRITE_KEY, config);
194+
195+
var TestCases = new RetryErrorTestCase[]
196+
{
197+
// The errors (500 > code >= 400) doesn't require retry
198+
new RetryErrorTestCase()
199+
{
200+
ErrorMessage = "Server Gone",
201+
ResponseCode = HttpStatusCode.Gone,
202+
ShouldRetry = false,
203+
Timeout = 10000
204+
},
205+
// 429 error requires retry
206+
new RetryErrorTestCase()
207+
{
208+
ErrorMessage = "Too many requests",
209+
ResponseCode = (HttpStatusCode)429,
210+
ShouldRetry = true,
211+
Timeout = 10000
212+
},
213+
// Server errors require retry
214+
new RetryErrorTestCase()
215+
{
216+
ErrorMessage = "Bad Gateway",
217+
ResponseCode = HttpStatusCode.BadGateway,
218+
ShouldRetry = true,
219+
Timeout = 10000
220+
}
221+
};
222+
223+
foreach (var testCase in TestCases)
224+
{
225+
// Setup fallback module which returns error code
226+
DummyServer.RequestHandler = ((req, res) =>
227+
{
228+
string pageData = "{ ErrorMessage: '" + testCase.ErrorMessage + "' }";
229+
byte[] data = Encoding.UTF8.GetBytes(pageData);
230+
231+
res.StatusCode = (int)testCase.ResponseCode;
232+
res.ContentType = "application/json";
233+
res.ContentEncoding = Encoding.UTF8;
234+
res.ContentLength64 = data.LongLength;
235+
236+
res.OutputStream.Write(data, 0, data.Length);
237+
res.Close();
238+
});
239+
240+
// Calculate working time for Identiy message with invalid host address
241+
watch.Start();
242+
Actions.Identify(Analytics.Client);
243+
watch.Stop();
244+
245+
DummyServer.RequestHandler = null;
246+
247+
Assert.AreEqual(0, Analytics.Client.Statistics.Succeeded);
248+
249+
// Handling Identify message will less than 10s because the server returns GONE message.
250+
// That's because it retries submit when it's failed.
251+
if (testCase.ShouldRetry)
252+
Assert.IsTrue(watch.ElapsedMilliseconds > testCase.Timeout);
253+
else
254+
Assert.IsFalse(watch.ElapsedMilliseconds > testCase.Timeout);
255+
}
256+
}
257+
}
153258

154259
[Test()]
155260
public void ProxyTestNet35()

Test.Net45/ConnectionTests.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,32 @@ public void RetryErrorTestNet45()
4242
{
4343
Stopwatch watch = new Stopwatch();
4444

45+
// Set invalid host address and make timeout to 1s
46+
var config = new Config().SetAsync(false);
47+
config.SetHost("https://fake.segment-server.com");
48+
config.SetTimeout(new TimeSpan(0, 0, 1));
49+
config.SetMaxRetryTime(new TimeSpan(0, 0, 10));
50+
Analytics.Initialize(Constants.WRITE_KEY, config);
51+
52+
// Calculate working time for Identiy message with invalid host address
53+
watch.Start();
54+
Actions.Identify(Analytics.Client);
55+
watch.Stop();
56+
57+
Assert.AreEqual(1, Analytics.Client.Statistics.Submitted);
58+
Assert.AreEqual(0, Analytics.Client.Statistics.Succeeded);
59+
Assert.AreEqual(1, Analytics.Client.Statistics.Failed);
60+
61+
// Handling Identify message will take more than 10s even though the timeout is 1s.
62+
// That's because it retries submit when it's failed.
63+
Assert.AreEqual(true, watch.ElapsedMilliseconds > 10000);
64+
}
65+
66+
[Test()]
67+
public void RetryErrorWithDefaultMaxRetryTimeTestNet45()
68+
{
69+
Stopwatch watch = new Stopwatch();
70+
4571
// Set invalid host address and make timeout to 1s
4672
var config = new Config().SetAsync(false);
4773
config.SetHost("https://fake.segment-server.com");

0 commit comments

Comments
 (0)