diff --git a/src/lib/Microsoft.Health.Extensions.Fhir.R4/FhirClientFactory.cs b/src/lib/Microsoft.Health.Extensions.Fhir.R4/FhirClientFactory.cs index 67fca95d..f5c4772a 100644 --- a/src/lib/Microsoft.Health.Extensions.Fhir.R4/FhirClientFactory.cs +++ b/src/lib/Microsoft.Health.Extensions.Fhir.R4/FhirClientFactory.cs @@ -76,19 +76,17 @@ private static FhirClient CreateClient(TokenCredential tokenCredential, ITelemet PreferredFormat = ResourceFormat.Json, }; - FhirClient client; + FhirClient client = null; try { client = new FhirClient(url, fhirClientSettings, new BearerTokenAuthorizationMessageHandler(uri, tokenCredential, logger)); + FhirServiceValidator.ValidateFhirService(client, logger); } catch (Exception ex) { - client = null; FhirServiceExceptionProcessor.ProcessException(ex, logger); } - FhirServiceValidator.ValidateFhirService(client, logger); - return client; } diff --git a/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceErrorCode.cs b/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceErrorCode.cs index 77422568..8fb54207 100644 --- a/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceErrorCode.cs +++ b/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceErrorCode.cs @@ -17,6 +17,21 @@ public enum FhirServiceErrorCode /// AuthorizationError, + /// + /// Error code that categorizes invalid arguments (i.e. exceptions encountered of type ArgumentException), which may occur when FhirClient's endpoint is validated + /// + ArgumentError, + + /// + /// Error code that categorizes HTTP request exceptions (i.e. exceptions encountered of type HttpRequestException) + /// + HttpRequestError, + + /// + /// Error code that categorizes MSAL.NET exceptions (i.e. exceptions encountered of type MsalServiceException) + /// + MsalServiceError, + /// /// Error code that categorizes all other generic exceptions /// diff --git a/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceExceptionProcessor.cs b/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceExceptionProcessor.cs index 3d15bff9..4e935e63 100644 --- a/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceExceptionProcessor.cs +++ b/src/lib/Microsoft.Health.Extensions.Fhir.R4/Telemetry/Exceptions/FhirServiceExceptionProcessor.cs @@ -58,21 +58,44 @@ public static (Exception customException, string errorName) CustomizeException(E return (exception, status.ToString()); } + case ArgumentException _: + var paramName = ((ArgumentException)exception).ParamName; + if (paramName.Contains("endpoint", StringComparison.OrdinalIgnoreCase)) + { + message = FhirResources.FhirServiceEndpointInvalid; + errorName = nameof(FhirServiceErrorCode.ConfigurationError); + return (new InvalidFhirServiceException(message, exception, errorName), errorName); + } + + return (exception, $"{nameof(FhirServiceErrorCode.ArgumentError)}_{paramName}"); + case UriFormatException _: message = FhirResources.FhirServiceUriFormatInvalid; errorName = nameof(FhirServiceErrorCode.ConfigurationError); return (new InvalidFhirServiceException(message, exception, errorName), errorName); case HttpRequestException _: - message = FhirResources.FhirServiceHttpRequestError; - errorName = nameof(FhirServiceErrorCode.ConfigurationError); - return (new InvalidFhirServiceException(message, exception, errorName), errorName); + // TODO: In .NET 5 and later, check HttpRequestException's StatusCode property instead of the Message property + if (exception.Message.Contains(FhirResources.HttpRequestErrorNotKnown, StringComparison.CurrentCultureIgnoreCase)) + { + message = FhirResources.FhirServiceHttpRequestError; + errorName = nameof(FhirServiceErrorCode.ConfigurationError); + return (new InvalidFhirServiceException(message, exception, errorName), errorName); + } + + return (exception, $"{nameof(FhirServiceErrorCode.HttpRequestError)}"); case MsalServiceException _: - string errorCode = ((MsalServiceException)exception).ErrorCode; - message = FhirResources.FhirServiceMsalServiceError; - errorName = $"{nameof(FhirServiceErrorCode.ConfigurationError)}_{errorCode}"; - return (new InvalidFhirServiceException(message, exception, errorName), errorName); + var errorCode = ((MsalServiceException)exception).ErrorCode; + if (string.Equals(errorCode, "invalid_resource", StringComparison.OrdinalIgnoreCase) + || string.Equals(errorCode, "invalid_scope", StringComparison.OrdinalIgnoreCase)) + { + message = FhirResources.FhirServiceMsalServiceError; + errorName = $"{nameof(FhirServiceErrorCode.ConfigurationError)}"; + return (new InvalidFhirServiceException(message, exception, errorName), errorName); + } + + return (exception, $"{nameof(FhirServiceErrorCode.MsalServiceError)}_{errorCode}"); default: return (exception, nameof(FhirServiceErrorCode.GeneralError)); diff --git a/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.Designer.cs b/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.Designer.cs index 85da1104..8f584087 100644 --- a/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.Designer.cs +++ b/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.Designer.cs @@ -69,6 +69,15 @@ public static string FhirServiceAccessForbidden { } } + /// + /// Looks up a localized string similar to Verify that the FHIR service URL is provided and is an absolute URL.. + /// + public static string FhirServiceEndpointInvalid { + get { + return ResourceManager.GetString("FhirServiceEndpointInvalid", resourceCulture); + } + } + /// /// Looks up a localized string similar to Verify that the provided FHIR service URL exists.. /// @@ -104,5 +113,14 @@ public static string FhirServiceUriFormatInvalid { return ResourceManager.GetString("FhirServiceUriFormatInvalid", resourceCulture); } } + + /// + /// Looks up a localized string similar to Name or service not known. + /// + public static string HttpRequestErrorNotKnown { + get { + return ResourceManager.GetString("HttpRequestErrorNotKnown", resourceCulture); + } + } } } diff --git a/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.resx b/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.resx index c668be32..c2ea960c 100644 --- a/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.resx +++ b/src/lib/Microsoft.Health.Extensions.Fhir/Resources/FhirResources.resx @@ -120,6 +120,9 @@ Verify that the provided FHIR service's 'FHIR Data Writer' role has been assigned to the applicable Azure Active Directory security principal or managed identity. + + Verify that the FHIR service URL is provided and is an absolute URL. + Verify that the provided FHIR service URL exists. @@ -132,4 +135,7 @@ Verify that the provided FHIR service URL is an absolute URL. + + Name or service not known + \ No newline at end of file diff --git a/test/Microsoft.Health.Extensions.Fhir.R4.UnitTests/FhirServiceExceptionProcessorTests.cs b/test/Microsoft.Health.Extensions.Fhir.R4.UnitTests/FhirServiceExceptionProcessorTests.cs index 5be54f1b..71c73e5c 100644 --- a/test/Microsoft.Health.Extensions.Fhir.R4.UnitTests/FhirServiceExceptionProcessorTests.cs +++ b/test/Microsoft.Health.Extensions.Fhir.R4.UnitTests/FhirServiceExceptionProcessorTests.cs @@ -22,8 +22,14 @@ public class FhirServiceExceptionProcessorTests private static readonly Exception _fhirForbiddenEx = new FhirOperationException("test", HttpStatusCode.Forbidden); private static readonly Exception _fhirNotFoundEx = new FhirOperationException("test", HttpStatusCode.NotFound); private static readonly Exception _fhirBadRequestEx = new FhirOperationException("test", HttpStatusCode.BadRequest); + private static readonly Exception _argEndpointNullEx = new ArgumentNullException("endpoint"); + private static readonly Exception _argEndpointEx = new ArgumentException("endpoint", "Endpoint must be absolute"); + private static readonly Exception _argEx = new ArgumentException("test_message", "test_param"); private static readonly Exception _uriEx = new UriFormatException(); + private static readonly Exception _httpNotKnownEx = new HttpRequestException("Name or service not known"); private static readonly Exception _httpEx = new HttpRequestException(); + private static readonly Exception _msalInvalidResourceEx = new MsalServiceException("invalid_resource", "test_message"); + private static readonly Exception _msalInvalidScopeEx = new MsalServiceException("invalid_scope", "test_message"); private static readonly Exception _msalEx = new MsalServiceException("test_code", "test_message"); private static readonly Exception _ex = new Exception(); @@ -33,9 +39,15 @@ public class FhirServiceExceptionProcessorTests new object[] { _fhirForbiddenEx, "FHIRServiceErrorAuthorizationError" }, new object[] { _fhirNotFoundEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, new object[] { _fhirBadRequestEx, "FHIRServiceErrorBadRequest" }, + new object[] { _argEndpointNullEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, + new object[] { _argEndpointEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, + new object[] { _argEx, "FHIRServiceErrorArgumentError_test_param" }, new object[] { _uriEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, - new object[] { _httpEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, - new object[] { _msalEx, "FHIRServiceErrorConfigurationError_test_code", nameof(ErrorSource.User) }, + new object[] { _httpNotKnownEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, + new object[] { _httpEx, "FHIRServiceErrorHttpRequestError" }, + new object[] { _msalInvalidResourceEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, + new object[] { _msalInvalidScopeEx, "FHIRServiceErrorConfigurationError", nameof(ErrorSource.User) }, + new object[] { _msalEx, "FHIRServiceErrorMsalServiceError_test_code" }, new object[] { _ex, "FHIRServiceErrorGeneralError" }, }; @@ -45,9 +57,15 @@ public class FhirServiceExceptionProcessorTests new object[] { _fhirForbiddenEx, typeof(UnauthorizedAccessFhirServiceException) }, new object[] { _fhirNotFoundEx, typeof(InvalidFhirServiceException) }, new object[] { _fhirBadRequestEx, typeof(FhirOperationException) }, + new object[] { _argEndpointNullEx, typeof(InvalidFhirServiceException) }, + new object[] { _argEndpointEx, typeof(InvalidFhirServiceException) }, + new object[] { _argEx, typeof(ArgumentException) }, new object[] { _uriEx, typeof(InvalidFhirServiceException) }, - new object[] { _httpEx, typeof(InvalidFhirServiceException) }, - new object[] { _msalEx, typeof(InvalidFhirServiceException) }, + new object[] { _httpNotKnownEx, typeof(InvalidFhirServiceException) }, + new object[] { _httpEx, typeof(HttpRequestException) }, + new object[] { _msalInvalidResourceEx, typeof(InvalidFhirServiceException) }, + new object[] { _msalInvalidScopeEx, typeof(InvalidFhirServiceException) }, + new object[] { _msalEx, typeof(MsalServiceException) }, new object[] { _ex, typeof(Exception) }, };