Skip to content

Commit 226269b

Browse files
hiranya911hiranya911
committed
Support for handling non json responses
1 parent 41e7bf1 commit 226269b

File tree

10 files changed

+128
-35
lines changed

10 files changed

+128
-35
lines changed

‎FirebaseAdmin/FirebaseAdmin.Tests/HttpErrorHandlerTest.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,28 @@ public void KnownHttpStatusCode(HttpStatusCode statusCode, ErrorCode expected)
5757
Assert.Null(error.InnerException);
5858
}
5959

60+
[Theory]
61+
[MemberData(nameof(HttpErrorCodes))]
62+
public void NonJsonResponse(HttpStatusCode statusCode, ErrorCode expected)
63+
{
64+
var text = "not json";
65+
var resp = new HttpResponseMessage()
66+
{
67+
StatusCode = statusCode,
68+
Content = new StringContent(text, Encoding.UTF8, "text/plain"),
69+
};
70+
71+
var handler = new HttpErrorHandler();
72+
var error = Assert.Throws<FirebaseException>(() => handler.ThrowIfError(resp, text));
73+
74+
Assert.Equal(expected, error.ErrorCode);
75+
Assert.Equal(
76+
$"Unexpected HTTP response with status: {(int)statusCode} ({statusCode})\n{text}",
77+
error.Message);
78+
Assert.Same(resp, error.HttpResponse);
79+
Assert.Null(error.InnerException);
80+
}
81+
6082
[Fact]
6183
public void UnknownHttpStatusCode()
6284
{

‎FirebaseAdmin/FirebaseAdmin.Tests/Messaging/BatchResponseTest.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,7 @@ public void SomeResponse()
4242
SendResponse.FromMessageId("message1"),
4343
SendResponse.FromMessageId("message2"),
4444
SendResponse.FromException(
45-
new FirebaseException(
46-
"error-message",
47-
null)),
45+
new FirebaseMessagingException(ErrorCode.Unknown, "error-message")),
4846
};
4947

5048
var batchResponse = new BatchResponse(responses);

‎FirebaseAdmin/FirebaseAdmin.Tests/Messaging/FirebaseMessagingClientTest.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,17 @@ public async Task HttpErrorAsync()
291291
{
292292
Topic = "test-topic",
293293
};
294-
var ex = await Assert.ThrowsAsync<FirebaseException>(
294+
295+
var ex = await Assert.ThrowsAsync<FirebaseMessagingException>(
295296
async () => await client.SendAsync(message));
296-
Assert.Contains("not json", ex.Message);
297+
298+
Assert.Equal(ErrorCode.Internal, ex.ErrorCode);
299+
Assert.Equal(
300+
$"Unexpected HTTP response with status: {500} (InternalServerError)\nnot json",
301+
ex.Message);
302+
Assert.Null(ex.MessagingErrorCode);
303+
Assert.NotNull(ex.HttpResponse);
304+
297305
var req = JsonConvert.DeserializeObject<FirebaseMessagingClient.SendRequest>(
298306
handler.LastRequestBody);
299307
Assert.Equal("test-topic", req.Message.Topic);

‎FirebaseAdmin/FirebaseAdmin.Tests/Messaging/SendResponseTest.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,7 @@ public void SuccessfulResponse()
3333
[Fact]
3434
public void FailureResponse()
3535
{
36-
var exception = new FirebaseException(
37-
"error-message",
38-
null);
36+
var exception = new FirebaseMessagingException(ErrorCode.Unknown, "error-message");
3937
var response = SendResponse.FromException(exception);
4038

4139
Assert.Null(response.MessageId);

‎FirebaseAdmin/FirebaseAdmin.Tests/PlatformErrorHandlerTest.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,27 @@ public void PlatformError()
4545
Assert.Null(error.InnerException);
4646
}
4747

48+
[Fact]
49+
public void NonJsonResponse()
50+
{
51+
var text = "not json";
52+
var resp = new HttpResponseMessage()
53+
{
54+
StatusCode = HttpStatusCode.ServiceUnavailable,
55+
Content = new StringContent(text, Encoding.UTF8, "text/plain"),
56+
};
57+
58+
var handler = new PlatformErrorHandler();
59+
var error = Assert.Throws<FirebaseException>(() => handler.ThrowIfError(resp, text));
60+
61+
Assert.Equal(ErrorCode.Unavailable, error.ErrorCode);
62+
Assert.Equal(
63+
$"Unexpected HTTP response with status: 503 (ServiceUnavailable)\n{text}",
64+
error.Message);
65+
Assert.Same(resp, error.HttpResponse);
66+
Assert.Null(error.InnerException);
67+
}
68+
4869
[Fact]
4970
public void PlatformErrorWithoutCode()
5071
{

‎FirebaseAdmin/FirebaseAdmin/HttpErrorHandler.cs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
namespace FirebaseAdmin
2121
{
22+
/// <summary>
23+
/// Base class for handling HTTP error responses.
24+
/// </summary>
2225
internal class HttpErrorHandler
2326
{
2427
private static readonly IReadOnlyDictionary<HttpStatusCode, ErrorCode> HttpErrorCodes =
@@ -34,25 +37,33 @@ internal class HttpErrorHandler
3437
{ HttpStatusCode.ServiceUnavailable, ErrorCode.Unavailable },
3538
};
3639

37-
internal void ThrowIfError(HttpResponseMessage response, string json)
40+
/// <summary>
41+
/// Parses the given HTTP response and throws a <see cref="FirebaseException"/> if it
42+
/// contains an error. Does nothing if the status code of the HTTP response indicates
43+
/// success.
44+
/// </summary>
45+
/// <param name="response">HTTP response message to process.</param>
46+
/// <param name="body">The response body read from the message.</param>
47+
/// <exception cref="FirebaseException">If the response contains an error.</exception>
48+
internal void ThrowIfError(HttpResponseMessage response, string body)
3849
{
3950
if (response.IsSuccessStatusCode)
4051
{
4152
return;
4253
}
4354

44-
var info = this.ExtractErrorInfo(response, json);
55+
var info = this.ExtractErrorInfo(response, body);
4556
var args = new FirebaseExceptionArgs()
4657
{
4758
Code = info.Code,
4859
Message = info.Message,
4960
HttpResponse = response,
50-
ResponseBody = json,
61+
ResponseBody = body,
5162
};
5263
throw this.CreateException(args);
5364
}
5465

55-
protected virtual ErrorInfo ExtractErrorInfo(HttpResponseMessage response, string json)
66+
protected virtual ErrorInfo ExtractErrorInfo(HttpResponseMessage response, string body)
5667
{
5768
ErrorCode code;
5869
if (!HttpErrorCodes.TryGetValue(response.StatusCode, out code))
@@ -62,7 +73,7 @@ protected virtual ErrorInfo ExtractErrorInfo(HttpResponseMessage response, strin
6273

6374
var message = "Unexpected HTTP response with status: "
6475
+ $"{(int)response.StatusCode} ({response.StatusCode})"
65-
+ $"{Environment.NewLine}{json}";
76+
+ $"{Environment.NewLine}{body}";
6677
return new ErrorInfo()
6778
{
6879
Code = code,

‎FirebaseAdmin/FirebaseAdmin/Messaging/FirebaseMessagingClient.cs

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ internal sealed class FirebaseMessagingClient : IDisposable
4242
private readonly string sendUrl;
4343
private readonly string restPath;
4444
private readonly FCMClientService fcmClientService;
45+
private readonly HttpErrorHandler errorHandler;
4546

4647
public FirebaseMessagingClient(
4748
HttpClientFactory clientFactory, GoogleCredential credential, string projectId)
@@ -60,6 +61,7 @@ public FirebaseMessagingClient(
6061
this.sendUrl = string.Format(FcmSendUrl, projectId);
6162
this.restPath = this.sendUrl.Substring(FcmBaseUrl.Length);
6263
this.fcmClientService = new FCMClientService(this.httpClient);
64+
this.errorHandler = new MessagingErrorHandler();
6365
}
6466

6567
internal static string ClientVersion
@@ -103,14 +105,7 @@ public async Task<string> SendAsync(
103105
{
104106
var response = await this.SendRequestAsync(request, cancellationToken).ConfigureAwait(false);
105107
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
106-
if (!response.IsSuccessStatusCode)
107-
{
108-
// TODO(hkj): Handle error responses.
109-
var error = "Response status code does not indicate success: "
110-
+ $"{(int)response.StatusCode} ({response.StatusCode})"
111-
+ $"{Environment.NewLine}{json}";
112-
throw new FirebaseException(error);
113-
}
108+
this.errorHandler.ThrowIfError(response, json);
114109

115110
var parsed = JsonConvert.DeserializeObject<SingleMessageResponse>(json);
116111
return parsed.Name;
@@ -157,7 +152,7 @@ public async Task<BatchResponse> SendAllAsync(
157152
}
158153
catch (HttpRequestException e)
159154
{
160-
throw new FirebaseException("Error while calling the FCM service.", e);
155+
throw e.ToFirebaseException();
161156
}
162157
}
163158

@@ -171,9 +166,18 @@ private static void AddCommonHeaders(HttpRequestMessage request)
171166
request.Headers.Add("X-Firebase-Client", ClientVersion);
172167
}
173168

174-
private static FirebaseException CreateExceptionFor(RequestError requestError)
169+
private async Task<FirebaseMessagingException> CreateExceptionFor(HttpResponseMessage response)
175170
{
176-
return new FirebaseException(requestError.ToString());
171+
var json = await response.Content.ReadAsStringAsync();
172+
try
173+
{
174+
this.errorHandler.ThrowIfError(response, json);
175+
return null;
176+
}
177+
catch (FirebaseMessagingException ex)
178+
{
179+
return ex;
180+
}
177181
}
178182

179183
private async Task<HttpResponseMessage> SendRequestAsync(object body, CancellationToken cancellationToken)
@@ -198,20 +202,22 @@ private async Task<BatchResponse> SendBatchRequestAsync(
198202
var batch = this.CreateBatchRequest(
199203
messages,
200204
dryRun,
201-
(content, error, index, message) =>
205+
async (content, error, index, message) =>
202206
{
203207
if (error != null)
204208
{
205-
responses.Add(SendResponse.FromException(CreateExceptionFor(error)));
209+
responses.Add(SendResponse.FromException(await this.CreateExceptionFor(message)));
206210
}
207211
else if (content != null)
208212
{
209213
responses.Add(SendResponse.FromMessageId(content.Name));
210214
}
211215
else
212216
{
213-
responses.Add(SendResponse.FromException(new FirebaseException(
214-
$"Unexpected batch response. Response status code was {message.StatusCode}.")));
217+
var exception = new FirebaseMessagingException(
218+
ErrorCode.Unknown,
219+
$"Unexpected batch response. Response status code: {message.StatusCode}.");
220+
responses.Add(SendResponse.FromException(exception));
215221
}
216222
});
217223

‎FirebaseAdmin/FirebaseAdmin/Messaging/MessagingErrorHandler.cs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,16 @@
1414

1515
using System.Collections.Generic;
1616
using System.Linq;
17+
using System.Net.Http;
1718
using Google.Apis.Json;
1819
using Newtonsoft.Json;
1920

2021
namespace FirebaseAdmin.Messaging
2122
{
23+
/// <summary>
24+
/// Parses error responses received from the FCM service, and creates instances of
25+
/// <see cref="FirebaseMessagingException"/>.
26+
/// </summary>
2227
internal sealed class MessagingErrorHandler : PlatformErrorHandler
2328
{
2429
private static readonly string MessagingErrorType =
@@ -39,15 +44,27 @@ internal sealed class MessagingErrorHandler : PlatformErrorHandler
3944

4045
protected override FirebaseException CreateException(FirebaseExceptionArgs args)
4146
{
42-
var fcmError = NewtonsoftJsonSerializer.Instance.Deserialize<MessagingErrorResponse>(
43-
args.ResponseBody);
4447
return new FirebaseMessagingException(
4548
args.Code,
4649
args.Message,
47-
fcmError.Error?.GetMessagingErrorCode(),
50+
this.GetMessagingErrorCode(args.ResponseBody),
4851
response: args.HttpResponse);
4952
}
5053

54+
private MessagingErrorCode? GetMessagingErrorCode(string body)
55+
{
56+
try
57+
{
58+
var fcmError = NewtonsoftJsonSerializer.Instance.Deserialize<MessagingErrorResponse>(
59+
body);
60+
return fcmError.Error?.GetMessagingErrorCode();
61+
}
62+
catch
63+
{
64+
return null;
65+
}
66+
}
67+
5168
private sealed class MessagingErrorResponse
5269
{
5370
[JsonProperty("error")]

‎FirebaseAdmin/FirebaseAdmin/Messaging/SendResponse.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ internal static SendResponse FromMessageId(string messageId)
6060
return new SendResponse(messageId);
6161
}
6262

63-
internal static SendResponse FromException(FirebaseException exception)
63+
internal static SendResponse FromException(FirebaseMessagingException exception)
6464
{
6565
if (exception == null)
6666
{

‎FirebaseAdmin/FirebaseAdmin/PlatformErrorHandler.cs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ internal class PlatformErrorHandler : HttpErrorHandler
3131
{ "UNAVAILABLE", ErrorCode.Unavailable },
3232
};
3333

34-
protected sealed override ErrorInfo ExtractErrorInfo(HttpResponseMessage response, string json)
34+
protected sealed override ErrorInfo ExtractErrorInfo(HttpResponseMessage response, string body)
3535
{
36-
var parsedResponse = NewtonsoftJsonSerializer.Instance.Deserialize<PlatformErrorResponse>(json);
36+
var parsedResponse = this.ParseResponseBody(body);
3737
var status = parsedResponse.Error?.Status ?? string.Empty;
38-
var defaults = base.ExtractErrorInfo(response, json);
38+
var defaults = base.ExtractErrorInfo(response, body);
3939

4040
ErrorCode code;
4141
if (!PlatformErrorCodes.TryGetValue(status, out code))
@@ -56,6 +56,18 @@ protected sealed override ErrorInfo ExtractErrorInfo(HttpResponseMessage respons
5656
};
5757
}
5858

59+
private PlatformErrorResponse ParseResponseBody(string body)
60+
{
61+
try
62+
{
63+
return NewtonsoftJsonSerializer.Instance.Deserialize<PlatformErrorResponse>(body);
64+
}
65+
catch
66+
{
67+
return new PlatformErrorResponse();
68+
}
69+
}
70+
5971
internal sealed class PlatformErrorResponse
6072
{
6173
[JsonProperty("error")]

0 commit comments

Comments
 (0)