Skip to content

Commit 45adbaa

Browse files
niemyjskiniemyjski
committed
Harden provider transport error handling
Root cause: the HttpClient transport cleanup left several provider failure paths under-tested and changed exception diagnostics/contracts in ways that made regressions easier to miss.
1 parent 3934bbf commit 45adbaa

13 files changed

Lines changed: 482 additions & 100 deletions

‎src/Geocoding.Here/HereGeocoder.cs‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,12 @@ private IEnumerable<HereAddress> ParseResponse(HereResponse response)
221221

222222
var address = item.Address ?? new HereAddressPayload();
223223
var coordinates = item.Access?.FirstOrDefault() ?? item.Position;
224+
var formattedAddress = FirstNonEmpty(address.Label, item.Title);
225+
if (String.IsNullOrWhiteSpace(formattedAddress))
226+
continue;
227+
224228
yield return new HereAddress(
225-
address.Label ?? item.Title ?? "",
229+
formattedAddress,
226230
new Location(coordinates.Lat, coordinates.Lng),
227231
address.Street,
228232
address.HouseNumber,
@@ -239,7 +243,11 @@ private HttpRequestMessage CreateRequest(Uri url)
239243
return new HttpRequestMessage(HttpMethod.Get, url);
240244
}
241245

242-
private HttpClient BuildClient()
246+
/// <summary>
247+
/// Builds the HTTP client used for HERE requests.
248+
/// </summary>
249+
/// <returns>The configured HTTP client.</returns>
250+
protected virtual HttpClient BuildClient()
243251
{
244252
if (Proxy is null)
245253
return new HttpClient();
@@ -256,11 +264,28 @@ private async Task<HereResponse> GetResponse(Uri queryUrl, CancellationToken can
256264
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
257265

258266
if (!response.IsSuccessStatusCode)
259-
throw new HereGeocodingException($"HERE request failed ({(int)response.StatusCode} {response.ReasonPhrase}): {json}", response.ReasonPhrase, ((int)response.StatusCode).ToString(CultureInfo.InvariantCulture));
267+
throw new HereGeocodingException($"HERE request failed ({(int)response.StatusCode} {response.ReasonPhrase}).{BuildResponsePreview(json)}", response.ReasonPhrase, ((int)response.StatusCode).ToString(CultureInfo.InvariantCulture));
260268

261269
return JsonSerializer.Deserialize<HereResponse>(json, Extensions.JsonOptions) ?? new HereResponse();
262270
}
263271

272+
private static string BuildResponsePreview(string? body)
273+
{
274+
if (String.IsNullOrWhiteSpace(body))
275+
return String.Empty;
276+
277+
var preview = body!.Trim();
278+
if (preview.Length > 256)
279+
preview = preview.Substring(0, 256) + "...";
280+
281+
return " Response preview: " + preview;
282+
}
283+
284+
private static string FirstNonEmpty(params string?[] values)
285+
{
286+
return values.FirstOrDefault(value => !String.IsNullOrWhiteSpace(value)) ?? String.Empty;
287+
}
288+
264289
private static HereLocationType MapLocationType(string? resultType)
265290
{
266291
switch (resultType?.Trim().ToLowerInvariant())

‎src/Geocoding.MapQuest/MapQuestGeocoder.cs‎

Lines changed: 50 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Net;
2+
using System.Net.Http;
23
using System.Text;
34

45
namespace Geocoding.MapQuest;
@@ -137,8 +138,9 @@ where l.Quality < Quality.COUNTRY
137138
/// <returns>The deserialized MapQuest response.</returns>
138139
public async Task<MapQuestResponse> Execute(BaseRequest f, CancellationToken cancellationToken = default(CancellationToken))
139140
{
140-
HttpWebRequest request = await Send(f, cancellationToken).ConfigureAwait(false);
141-
MapQuestResponse r = await Parse(request, cancellationToken).ConfigureAwait(false);
141+
using var client = BuildClient();
142+
using var request = CreateRequest(f);
143+
MapQuestResponse r = await Parse(client, request, cancellationToken).ConfigureAwait(false);
142144
if (r is not null && !r.Results.IsNullOrEmpty())
143145
{
144146
foreach (MapQuestResult o in r.Results)
@@ -161,100 +163,95 @@ where l.Quality < Quality.COUNTRY
161163
return r!;
162164
}
163165

164-
private async Task<HttpWebRequest> Send(BaseRequest f, CancellationToken cancellationToken)
166+
/// <summary>
167+
/// Builds the HTTP client used for MapQuest requests.
168+
/// </summary>
169+
/// <returns>The configured HTTP client.</returns>
170+
protected virtual HttpClient BuildClient()
171+
{
172+
if (Proxy is null)
173+
return new HttpClient();
174+
175+
return new HttpClient(new HttpClientHandler { Proxy = Proxy });
176+
}
177+
178+
private HttpRequestMessage CreateRequest(BaseRequest f)
165179
{
166180
if (f is null)
167181
throw new ArgumentNullException(nameof(f));
168182

169-
HttpWebRequest request;
170-
bool hasBody = false;
183+
Uri requestUri;
171184
switch (f.RequestVerb)
172185
{
173186
case "GET":
174187
case "DELETE":
175188
case "HEAD":
176189
{
177190
var u = $"{f.RequestUri}json={WebUtility.UrlEncode(f.RequestBody)}&";
178-
request = WebRequest.CreateHttp(u);
191+
requestUri = new Uri(u, UriKind.Absolute);
179192
}
180193
break;
181194
case "POST":
182195
case "PUT":
183196
default:
184197
{
185-
request = WebRequest.CreateHttp(f.RequestUri);
186-
hasBody = !String.IsNullOrWhiteSpace(f.RequestBody);
198+
requestUri = f.RequestUri;
187199
}
188200
break;
189201
}
190-
request.Method = f.RequestVerb;
191-
request.ContentType = "application/" + f.InputFormat + "; charset=utf-8";
192-
193-
if (Proxy is not null)
194-
request.Proxy = Proxy;
195202

196-
if (hasBody)
203+
var request = new HttpRequestMessage(new HttpMethod(f.RequestVerb), requestUri);
204+
if (!String.IsNullOrWhiteSpace(f.RequestBody)
205+
&& !String.Equals(f.RequestVerb, "GET", StringComparison.OrdinalIgnoreCase)
206+
&& !String.Equals(f.RequestVerb, "DELETE", StringComparison.OrdinalIgnoreCase)
207+
&& !String.Equals(f.RequestVerb, "HEAD", StringComparison.OrdinalIgnoreCase))
197208
{
198-
byte[] buffer = Encoding.UTF8.GetBytes(f.RequestBody);
199-
//request.Headers.ContentLength = buffer.Length;
200-
using (cancellationToken.Register(request.Abort, false))
201-
using (Stream rs = await request.GetRequestStreamAsync().ConfigureAwait(false))
202-
{
203-
cancellationToken.ThrowIfCancellationRequested();
204-
await rs.WriteAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
205-
await rs.FlushAsync(cancellationToken).ConfigureAwait(false);
206-
}
209+
request.Content = new StringContent(f.RequestBody, Encoding.UTF8, "application/" + f.InputFormat);
207210
}
211+
208212
return request;
209213
}
210214

211-
private async Task<MapQuestResponse> Parse(HttpWebRequest request, CancellationToken cancellationToken)
215+
private async Task<MapQuestResponse> Parse(HttpClient client, HttpRequestMessage request, CancellationToken cancellationToken)
212216
{
213-
if (request is null)
214-
throw new ArgumentNullException(nameof(request));
215-
216217
string requestInfo = $"[{request.Method}] {request.RequestUri}";
217218
try
218219
{
219-
string json;
220-
using (HttpWebResponse response = (HttpWebResponse)await request.GetResponseAsync().ConfigureAwait(false))
221-
{
222-
cancellationToken.ThrowIfCancellationRequested();
223-
if ((int)response.StatusCode >= 300) //error
224-
throw new Exception((int)response.StatusCode + " " + response.StatusDescription);
220+
using var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false);
221+
cancellationToken.ThrowIfCancellationRequested();
222+
223+
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
224+
225+
if (!response.IsSuccessStatusCode)
226+
throw new Exception($"{(int)response.StatusCode} {requestInfo} | {response.ReasonPhrase}{BuildResponsePreview(json)}");
225227

226-
using (var sr = new StreamReader(response.GetResponseStream()!))
227-
json = await sr.ReadToEndAsync().ConfigureAwait(false);
228-
}
229228
if (String.IsNullOrWhiteSpace(json))
230229
throw new Exception("Remote system response with blank: " + requestInfo);
231230

232231
MapQuestResponse? o = json.FromJSON<MapQuestResponse>();
233232
if (o is null)
234-
throw new Exception("Unable to deserialize remote response: " + requestInfo + " => " + json);
233+
throw new Exception("Unable to deserialize remote response: " + requestInfo);
235234

236235
return o;
237236
}
238-
catch (WebException wex) //convert to simple exception & close the response stream
237+
catch (HttpRequestException ex)
239238
{
240-
if (wex.Response is not HttpWebResponse response)
241-
throw new Exception($"{requestInfo} | {wex.Status} | {wex.Message}", wex);
242-
243-
using (response)
244-
{
245-
var sb = new StringBuilder(requestInfo);
246-
sb.Append(" | ");
247-
sb.Append(response.StatusDescription);
248-
sb.Append(" | ");
249-
using (var sr = new StreamReader(response.GetResponseStream()!))
250-
{
251-
sb.Append(await sr.ReadToEndAsync().ConfigureAwait(false));
252-
}
253-
throw new Exception((int)response.StatusCode + " " + sb.ToString());
254-
}
239+
throw new Exception($"{requestInfo} | {ex.Message}", ex);
255240
}
256241
}
257242

243+
private static string BuildResponsePreview(string? body)
244+
{
245+
if (String.IsNullOrWhiteSpace(body))
246+
return String.Empty;
247+
248+
var preview = body!.Trim();
249+
if (preview.Length > 256)
250+
preview = preview.Substring(0, 256) + "...";
251+
252+
return " | Response preview: " + preview;
253+
}
254+
258255
/// <inheritdoc />
259256
public async Task<IEnumerable<ResultItem>> GeocodeAsync(IEnumerable<string> addresses, CancellationToken cancellationToken = default(CancellationToken))
260257
{

‎src/Geocoding.Microsoft/AzureMapsGeocoder.cs‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,17 +202,24 @@ private async Task<AzureSearchResponse> GetResponseAsync(Uri queryUrl, Cancellat
202202
using (var request = new HttpRequestMessage(HttpMethod.Get, queryUrl))
203203
using (var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false))
204204
{
205-
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
206-
207205
if (!response.IsSuccessStatusCode)
208-
throw new AzureMapsGeocodingException($"Azure Maps request failed ({(int)response.StatusCode} {response.ReasonPhrase}): {json}");
206+
{
207+
var body = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
208+
throw new AzureMapsGeocodingException($"Azure Maps request failed ({(int)response.StatusCode} {response.ReasonPhrase}).{BuildResponsePreview(body)}");
209+
}
210+
211+
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
209212

210213
var payload = JsonSerializer.Deserialize<AzureSearchResponse>(json, Extensions.JsonOptions);
211214
return payload ?? new AzureSearchResponse();
212215
}
213216
}
214217

215-
private HttpClient BuildClient()
218+
/// <summary>
219+
/// Builds the HTTP client used for Azure Maps requests.
220+
/// </summary>
221+
/// <returns>The configured HTTP client.</returns>
222+
protected virtual HttpClient BuildClient()
216223
{
217224
if (Proxy is null)
218225
return new HttpClient();
@@ -225,9 +232,10 @@ private IEnumerable<AzureMapsAddress> ParseResponse(AzureSearchResponse response
225232
{
226233
if (response.Results is not null && response.Results.Length > 0)
227234
{
228-
foreach (var result in response.Results.Where(result => result?.Position is not null))
235+
foreach (var azureResult in response.Results
236+
.Where(result => result?.Position is not null)
237+
.Select(result => result!))
229238
{
230-
var azureResult = result!;
231239
var address = azureResult.Address ?? new AzureAddressPayload();
232240
var formattedAddress = FirstNonEmpty(address.FreeformAddress, address.StreetNameAndNumber, BuildStreetLine(address.StreetNumber, address.StreetName), azureResult.Poi?.Name, azureResult.Type, FirstNonEmpty(address.LocalName, address.Municipality, address.CountryTertiarySubdivision), address.Country);
233241
if (String.IsNullOrWhiteSpace(formattedAddress))
@@ -314,6 +322,18 @@ private static string BuildStreetLine(string? streetNumber, string? streetName)
314322
return parts.Length == 0 ? String.Empty : String.Join(" ", parts);
315323
}
316324

325+
private static string BuildResponsePreview(string? body)
326+
{
327+
if (String.IsNullOrWhiteSpace(body))
328+
return String.Empty;
329+
330+
var preview = body!.Trim();
331+
if (preview.Length > 256)
332+
preview = preview.Substring(0, 256) + "...";
333+
334+
return " Response preview: " + preview;
335+
}
336+
317337
private static string FirstNonEmpty(params string?[] values)
318338
{
319339
return values.FirstOrDefault(value => !String.IsNullOrWhiteSpace(value)) ?? String.Empty;

‎src/Geocoding.Microsoft/BingGeocodingException.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,11 @@ public class BingGeocodingException : GeocodingException
1515
/// <param name="innerException">The underlying provider exception.</param>
1616
public BingGeocodingException(Exception innerException)
1717
: base(DefaultMessage, innerException) { }
18+
19+
/// <summary>
20+
/// Initializes a new instance of the <see cref="BingGeocodingException"/> class.
21+
/// </summary>
22+
/// <param name="message">The provider error message.</param>
23+
public BingGeocodingException(string message)
24+
: base(message) { }
1825
}

‎src/Geocoding.Microsoft/BingMapsGeocoder.cs‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,11 @@ private HttpRequestMessage CreateRequest(string url)
286286
return new HttpRequestMessage(HttpMethod.Get, url);
287287
}
288288

289-
private HttpClient BuildClient()
289+
/// <summary>
290+
/// Builds the HTTP client used for Bing Maps requests.
291+
/// </summary>
292+
/// <returns>The configured HTTP client.</returns>
293+
protected virtual HttpClient BuildClient()
290294
{
291295
if (Proxy is null)
292296
return new HttpClient();
@@ -300,12 +304,13 @@ private HttpClient BuildClient()
300304
{
301305
using (var client = BuildClient())
302306
{
303-
using var response = await client.SendAsync(CreateRequest(queryUrl), cancellationToken).ConfigureAwait(false);
307+
using var request = CreateRequest(queryUrl);
308+
using var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false);
304309

305310
if (!response.IsSuccessStatusCode)
306311
{
307312
var body = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
308-
throw new Exception($"Bing Maps request failed ({(int)response.StatusCode} {response.ReasonPhrase}): {body}");
313+
throw new BingGeocodingException(new HttpRequestException($"Bing Maps request failed ({(int)response.StatusCode} {response.ReasonPhrase}).{BuildResponsePreview(body)}"));
309314
}
310315

311316
using (var stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false))
@@ -330,6 +335,18 @@ private ConfidenceLevel EvaluateConfidence(string? confidence)
330335
return ConfidenceLevel.Unknown;
331336
}
332337

338+
private static string BuildResponsePreview(string? body)
339+
{
340+
if (String.IsNullOrWhiteSpace(body))
341+
return String.Empty;
342+
343+
var preview = body!.Trim();
344+
if (preview.Length > 256)
345+
preview = preview.Substring(0, 256) + "...";
346+
347+
return " Response preview: " + preview;
348+
}
349+
333350
private string BingUrlEncode(string toEncode)
334351
{
335352
if (String.IsNullOrEmpty(toEncode))

0 commit comments

Comments
 (0)