Skip to content

Commit e1b816c

Browse files
committed
Harden MapQuest exception handling
Root cause: MapQuest transport errors still exposed key-bearing request URLs in exception context and used raw Exception instead of provider-specific exceptions.
1 parent 0f0c28c commit e1b816c

3 files changed

Lines changed: 53 additions & 7 deletions

File tree

src/Geocoding.MapQuest/MapQuestGeocoder.cs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ private HttpRequestMessage CreateRequest(BaseRequest f)
212212

213213
private async Task<MapQuestResponse> Parse(HttpClient client, HttpRequestMessage request, CancellationToken cancellationToken)
214214
{
215-
string requestInfo = $"[{request.Method}] {request.RequestUri}";
215+
string requestInfo = BuildRequestInfo(request);
216216
try
217217
{
218218
using var response = await client.SendAsync(request, cancellationToken).ConfigureAwait(false);
@@ -221,21 +221,32 @@ private async Task<MapQuestResponse> Parse(HttpClient client, HttpRequestMessage
221221
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
222222

223223
if (!response.IsSuccessStatusCode)
224-
throw new Exception($"{(int)response.StatusCode} {requestInfo} | {response.ReasonPhrase}{BuildResponsePreview(json)}");
224+
throw new MapQuestGeocodingException($"{(int)response.StatusCode} {requestInfo} | {response.ReasonPhrase}{BuildResponsePreview(json)}");
225225

226226
if (String.IsNullOrWhiteSpace(json))
227-
throw new Exception("Remote system response with blank: " + requestInfo);
227+
throw new MapQuestGeocodingException("Remote system response with blank: " + requestInfo);
228228

229229
MapQuestResponse? o = json.FromJson<MapQuestResponse>();
230230
if (o is null)
231-
throw new Exception("Unable to deserialize remote response: " + requestInfo);
231+
throw new MapQuestGeocodingException("Unable to deserialize remote response: " + requestInfo);
232232

233233
return o;
234234
}
235235
catch (HttpRequestException ex)
236236
{
237-
throw new Exception($"{requestInfo} | {ex.Message}", ex);
237+
throw new MapQuestGeocodingException($"{requestInfo} | {ex.Message}", ex);
238238
}
239+
catch (Exception ex) when (ex is not MapQuestGeocodingException)
240+
{
241+
throw new MapQuestGeocodingException(ex);
242+
}
243+
}
244+
245+
private static string BuildRequestInfo(HttpRequestMessage request)
246+
{
247+
string method = request.Method.Method;
248+
string requestUri = request.RequestUri?.GetLeftPart(UriPartial.Path) ?? "(unknown-uri)";
249+
return $"[{method}] {requestUri}";
239250
}
240251

241252
private static string BuildResponsePreview(string? body)
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using Geocoding.Core;
2+
3+
namespace Geocoding.MapQuest;
4+
5+
/// <summary>
6+
/// Represents an error returned by the MapQuest geocoding provider.
7+
/// </summary>
8+
public class MapQuestGeocodingException : GeocodingException
9+
{
10+
private const string DefaultMessage = "There was an error processing the geocoding request. See InnerException for more information.";
11+
12+
/// <summary>
13+
/// Initializes a new instance of the <see cref="MapQuestGeocodingException"/> class.
14+
/// </summary>
15+
/// <param name="innerException">The underlying provider exception.</param>
16+
public MapQuestGeocodingException(Exception innerException)
17+
: base(DefaultMessage, innerException) { }
18+
19+
/// <summary>
20+
/// Initializes a new instance of the <see cref="MapQuestGeocodingException"/> class.
21+
/// </summary>
22+
/// <param name="message">The provider error message.</param>
23+
public MapQuestGeocodingException(string message)
24+
: base(message) { }
25+
26+
/// <summary>
27+
/// Initializes a new instance of the <see cref="MapQuestGeocodingException"/> class.
28+
/// </summary>
29+
/// <param name="message">The provider error message.</param>
30+
/// <param name="innerException">The underlying provider exception.</param>
31+
public MapQuestGeocodingException(string message, Exception innerException)
32+
: base(message, innerException) { }
33+
}

test/Geocoding.Tests/MapQuestGeocoderTest.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,12 @@ public async Task Geocode_ConnectionFailure_IncludesRequestContext()
128128
var geocoder = new TestableMapQuestGeocoder(new TestHttpMessageHandler((_, _) => throw new HttpRequestException("Name or service not known")));
129129

130130
// Act
131-
var exception = await Assert.ThrowsAsync<Exception>(() => geocoder.GeocodeAsync("1600 pennsylvania ave nw, washington dc", TestContext.Current.CancellationToken));
131+
var exception = await Assert.ThrowsAsync<MapQuestGeocodingException>(() => geocoder.GeocodeAsync("1600 pennsylvania ave nw, washington dc", TestContext.Current.CancellationToken));
132132

133133
// Assert
134134
Assert.Contains("[POST]", exception.Message, StringComparison.Ordinal);
135135
Assert.Contains("mapquestapi.com/geocoding/v1/address", exception.Message, StringComparison.Ordinal);
136+
Assert.DoesNotContain("key=mapquest-key", exception.Message, StringComparison.Ordinal);
136137
Assert.IsType<HttpRequestException>(exception.InnerException);
137138
}
138139

@@ -144,11 +145,12 @@ public async Task Geocode_StatusFailure_UsesTrimmedPreviewMessage()
144145
var geocoder = new TestableMapQuestGeocoder(new TestHttpMessageHandler((_, _) => TestHttpMessageHandler.CreateResponseAsync(HttpStatusCode.BadGateway, "Bad Gateway", body)));
145146

146147
// Act
147-
var exception = await Assert.ThrowsAsync<Exception>(() => geocoder.GeocodeAsync("1600 pennsylvania ave nw, washington dc", TestContext.Current.CancellationToken));
148+
var exception = await Assert.ThrowsAsync<MapQuestGeocodingException>(() => geocoder.GeocodeAsync("1600 pennsylvania ave nw, washington dc", TestContext.Current.CancellationToken));
148149

149150
// Assert
150151
Assert.Contains("502", exception.Message, StringComparison.Ordinal);
151152
Assert.Contains("Response preview:", exception.Message, StringComparison.Ordinal);
153+
Assert.DoesNotContain("key=mapquest-key", exception.Message, StringComparison.Ordinal);
152154
Assert.DoesNotContain(body, exception.Message, StringComparison.Ordinal);
153155
}
154156

0 commit comments

Comments
 (0)