Skip to content

Commit

Permalink
Additional error logging for $import and $export (#4667)
Browse files Browse the repository at this point in the history
* adds extra foreach loop to capture JobStatus.Failed jobs

* Append the error file to the error details returned by the Import operation

* resolve conflict issue and review comment

---------

Co-authored-by: Anil Mahajan <[email protected]>
  • Loading branch information
mahajan-xor and Anil Mahajan authored Nov 15, 2024
1 parent f045d2f commit 449af6a
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Net;
using System.Threading;
Expand Down Expand Up @@ -65,25 +66,29 @@ public async Task WhenGettingFailedJob_ThenExecptionIsTrownWithCorrectResponseCo
{
var coord = new JobInfo() { Status = JobStatus.Completed };
var workerResult = new ImportJobErrorResult() { ErrorMessage = "Error", HttpStatusCode = statusCode };
var worker = new JobInfo() { Id = 1, Status = JobStatus.Failed, Result = JsonConvert.SerializeObject(workerResult) };
var worker = new JobInfo() { Id = 1, Status = JobStatus.Failed, Result = JsonConvert.SerializeObject(workerResult), Definition = JsonConvert.SerializeObject(new ImportProcessingJobDefinition() { ResourceLocation = "http://xyz" }) };
var definition = JsonConvert.DeserializeObject<ImportProcessingJobDefinition>(worker.Definition);
var resourceLocation = new Uri(definition.ResourceLocation);

var ofe = await Assert.ThrowsAsync<OperationFailedException>(() => SetupAndExecuteGetBulkImportJobByIdAsync(coord, [worker]));

Assert.Equal(statusCode == 0 ? HttpStatusCode.InternalServerError : statusCode, ofe.ResponseStatusCode);
Assert.Equal(string.Format(Core.Resources.OperationFailed, OperationsConstants.Import, ofe.ResponseStatusCode == HttpStatusCode.InternalServerError ? HttpStatusCode.InternalServerError : "Error"), ofe.Message);
Assert.Equal(string.Format(Core.Resources.OperationFailedWithErrorFile, OperationsConstants.Import, ofe.ResponseStatusCode == HttpStatusCode.InternalServerError ? HttpStatusCode.InternalServerError : "Error", resourceLocation.OriginalString), ofe.Message);
}

[Fact]
public async Task WhenGettingFailedJob_WithGenericException_ThenExecptionIsTrownWithCorrectResponseCode()
{
var coord = new JobInfo() { Status = JobStatus.Completed };
object workerResult = new { message = "Error", stackTrace = "Trace" };
var worker = new JobInfo() { Id = 1, Status = JobStatus.Failed, Result = JsonConvert.SerializeObject(workerResult) };
var worker = new JobInfo() { Id = 1, Status = JobStatus.Failed, Result = JsonConvert.SerializeObject(workerResult), Definition = JsonConvert.SerializeObject(new ImportProcessingJobDefinition() { ResourceLocation = "http://xyz" }) };
var definition = JsonConvert.DeserializeObject<ImportProcessingJobDefinition>(worker.Definition);
var resourceLocation = new Uri(definition.ResourceLocation);

var ofe = await Assert.ThrowsAsync<OperationFailedException>(() => SetupAndExecuteGetBulkImportJobByIdAsync(coord, [worker]));

Assert.Equal(HttpStatusCode.InternalServerError, ofe.ResponseStatusCode);
Assert.Equal(string.Format(Core.Resources.OperationFailed, OperationsConstants.Import, HttpStatusCode.InternalServerError), ofe.Message);
Assert.Equal(string.Format(Core.Resources.OperationFailedWithErrorFile, OperationsConstants.Import, HttpStatusCode.InternalServerError, resourceLocation.OriginalString), ofe.Message);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,18 @@ public async Task<GetImportResponse> Handle(GetImportRequest request, Cancellati
{
var failed = jobs.First(x => x.Status == JobStatus.Failed && !x.CancelRequested);
var errorResult = JsonConvert.DeserializeObject<ImportJobErrorResult>(failed.Result);
var definition = JsonConvert.DeserializeObject<ImportProcessingJobDefinition>(failed.Definition);
if (errorResult.HttpStatusCode == 0)
{
errorResult.HttpStatusCode = HttpStatusCode.InternalServerError;
}

var resourceLocation = new Uri(definition.ResourceLocation);

// hide error message for InternalServerError
var failureReason = errorResult.HttpStatusCode == HttpStatusCode.InternalServerError ? HttpStatusCode.InternalServerError.ToString() : errorResult.ErrorMessage;
throw new OperationFailedException(string.Format(Core.Resources.OperationFailed, OperationsConstants.Import, failureReason), errorResult.HttpStatusCode);

throw new OperationFailedException(string.Format(Core.Resources.OperationFailedWithErrorFile, OperationsConstants.Import, failureReason, resourceLocation.OriginalString), errorResult.HttpStatusCode);
}
else // no failures here
{
Expand Down
10 changes: 10 additions & 0 deletions src/Microsoft.Health.Fhir.Core/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/Microsoft.Health.Fhir.Core/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -747,4 +747,7 @@
<data name="CannotGetAccessToken" xml:space="preserve">
<value>Unable to get an access token for '{0}'.</value>
</data>
</root>
<data name="OperationFailedWithErrorFile" xml:space="preserve">
<value>{0} operation failed for reason: {1} ErrorFile: {2}</value>
</data>
</root>

0 comments on commit 449af6a

Please sign in to comment.