From 8c95c646c5a694a249fe90b479dca5a4a4830f74 Mon Sep 17 00:00:00 2001 From: Mariana Rios Flores Date: Tue, 2 Jun 2020 23:37:35 -0700 Subject: [PATCH 1/5] failures in LROs --- .../src/CopyModelOperation.cs | 17 ++++- .../src/FormRecognizerClient.cs | 16 ++--- .../src/FormTrainingClient.cs | 18 ++++-- .../src/RecognizeContentOperation.cs | 62 ++++++++++++++++-- .../src/RecognizeCustomFormsOperation.cs | 14 +++- .../src/RecognizeReceiptsOperation.cs | 64 +++++++++++++++++-- .../src/TrainingOperation.cs | 57 +++++++++++++++-- .../FormRecognizerClientLiveTests.cs | 5 +- .../FormTrainingClientLiveTests.cs | 18 +----- 9 files changed, 218 insertions(+), 53 deletions(-) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs index 2b3795da67660..1e2e631ece7a1 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs @@ -45,7 +45,18 @@ public class CopyModelOperation : Operation public override string Id { get; } /// - public override CustomFormModelInfo Value => OperationHelpers.GetValue(ref _value); + public override CustomFormModelInfo Value + { + get + { + if (HasCompleted && !HasValue) +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + throw new InvalidOperationException("Copy model operation failed"); +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + else + return OperationHelpers.GetValue(ref _value); + } + } /// public override bool HasCompleted => _hasCompleted; @@ -88,7 +99,7 @@ public CopyModelOperation(string operationId, string targetModelId, FormTraining /// Initializes a new instance of the class. /// /// The client for communicating with the Form Recognizer Azure Cognitive Service through its REST API. - /// Provides tools for exception creation in case of failure. + /// The client diagnostics for exception creation in case of failure. /// The address of the long-running operation. It can be obtained from the response headers upon starting the operation. /// Model id in the target Form Recognizer Resource. internal CopyModelOperation(ServiceRestClient serviceClient, ClientDiagnostics diagnostics, string operationLocation, string targetModelId) @@ -159,7 +170,7 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke else if (update.Value.Status == OperationStatus.Failed) { _hasCompleted = true; - _value = ConvertValue(update.Value, _targetModelId, CustomFormModelStatus.Invalid); + throw await CreateExceptionForFailedOperationAsync(async, update.Value.CopyResult.Errors).ConfigureAwait(false); } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs index eda371507227d..290fa9e24e637 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs @@ -133,7 +133,7 @@ public virtual RecognizeContentOperation StartRecognizeContent(Stream form, Reco FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(form, nameof(form)); ResponseWithHeaders response = ServiceClient.AnalyzeLayoutAsync(contentType, form, cancellationToken); - return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeContentOperation(ServiceClient, Diagnostics,response.Headers.OperationLocation); } /// @@ -153,7 +153,7 @@ public virtual async Task StartRecognizeContentAsync( FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(form, nameof(form)); ResponseWithHeaders response = await ServiceClient.AnalyzeLayoutAsyncAsync(contentType, form, cancellationToken).ConfigureAwait(false); - return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } /// @@ -171,7 +171,7 @@ public virtual RecognizeContentOperation StartRecognizeContentFromUri(Uri formUr SourcePath_internal sourcePath = new SourcePath_internal(formUrl.AbsoluteUri); ResponseWithHeaders response = ServiceClient.AnalyzeLayoutAsync(sourcePath, cancellationToken); - return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } /// @@ -189,7 +189,7 @@ public virtual async Task StartRecognizeContentFromUr SourcePath_internal sourcePath = new SourcePath_internal(formUrl.AbsoluteUri); ResponseWithHeaders response = await ServiceClient.AnalyzeLayoutAsyncAsync(sourcePath, cancellationToken).ConfigureAwait(false); - return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } #endregion @@ -213,7 +213,7 @@ public virtual async Task StartRecognizeReceiptsAsyn FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(receipt, nameof(receipt)); ResponseWithHeaders response = await ServiceClient.AnalyzeReceiptAsyncAsync(contentType, receipt, includeTextDetails: recognizeOptions.IncludeTextContent, cancellationToken).ConfigureAwait(false); - return new RecognizeReceiptsOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeReceiptsOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } /// @@ -233,7 +233,7 @@ public virtual RecognizeReceiptsOperation StartRecognizeReceipts(Stream receipt, FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(receipt, nameof(receipt)); ResponseWithHeaders response = ServiceClient.AnalyzeReceiptAsync(contentType, receipt, includeTextDetails: recognizeOptions.IncludeTextContent, cancellationToken); - return new RecognizeReceiptsOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeReceiptsOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } /// @@ -253,7 +253,7 @@ public virtual async Task StartRecognizeReceiptsFrom SourcePath_internal sourcePath = new SourcePath_internal(receiptUrl.AbsoluteUri); ResponseWithHeaders response = await ServiceClient.AnalyzeReceiptAsyncAsync(includeTextDetails: recognizeOptions.IncludeTextContent, sourcePath, cancellationToken).ConfigureAwait(false); - return new RecognizeReceiptsOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeReceiptsOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } /// @@ -273,7 +273,7 @@ public virtual RecognizeReceiptsOperation StartRecognizeReceiptsFromUri(Uri rece SourcePath_internal sourcePath = new SourcePath_internal(receiptUrl.AbsoluteUri); ResponseWithHeaders response = ServiceClient.AnalyzeReceiptAsync(includeTextDetails: recognizeOptions.IncludeTextContent, sourcePath, cancellationToken); - return new RecognizeReceiptsOperation(ServiceClient, response.Headers.OperationLocation); + return new RecognizeReceiptsOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } #endregion diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs index 30eac2980c53d..8fdb4a5c38021 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs @@ -111,8 +111,11 @@ public FormTrainingClient(Uri endpoint, TokenCredential credential, FormRecogniz /// If true, use a label file created in the <link-to-label-tool-doc> to provide training-time labels for training a model. If false, the model will be trained from forms only. /// Filter to apply to the documents in the source path for training. /// A controlling the request lifetime. - /// A to wait on this long-running operation. Its .Value upon successful - /// completion will contain meta-data about the trained model. + /// + /// A to wait on this long-running operation. Its .Value upon successful + /// completion will contain meta-data about the trained model. + /// If the training fails, an exception is raised, and a model with an "invalid" status is still created in the Form Recognizer resource. + /// [ForwardsClientCalls] public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTrainingLabels, TrainingFileFilter trainingFileFilter = default, CancellationToken cancellationToken = default) { @@ -121,7 +124,7 @@ public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTra var trainRequest = new TrainRequest_internal(trainingFilesUri.AbsoluteUri, trainingFileFilter, useTrainingLabels); ResponseWithHeaders response = ServiceClient.TrainCustomModelAsync(trainRequest); - return new TrainingOperation(response.Headers.Location, ServiceClient); + return new TrainingOperation(response.Headers.Location, ServiceClient, Diagnostics); } /// @@ -131,8 +134,11 @@ public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTra /// If true, use a label file created in the <link-to-label-tool-doc> to provide training-time labels for training a model. If false, the model will be trained from forms only. /// Filter to apply to the documents in the source path for training. /// A controlling the request lifetime. - /// A to wait on this long-running operation. Its .Value upon successful - /// completion will contain meta-data about the trained model. + /// + /// A to wait on this long-running operation. Its .Value upon successful + /// completion will contain meta-data about the trained model. + /// If the training fails, an exception is raised, and a model with an "invalid" status is still created in the Form Recognizer resource. + /// [ForwardsClientCalls] public virtual async Task StartTrainingAsync(Uri trainingFilesUri, bool useTrainingLabels, TrainingFileFilter trainingFileFilter = default, CancellationToken cancellationToken = default) { @@ -141,7 +147,7 @@ public virtual async Task StartTrainingAsync(Uri trainingFile var trainRequest = new TrainRequest_internal(trainingFilesUri.AbsoluteUri, trainingFileFilter, useTrainingLabels); ResponseWithHeaders response = await ServiceClient.TrainCustomModelAsyncAsync(trainRequest).ConfigureAwait(false); - return new TrainingOperation(response.Headers.Location, ServiceClient); + return new TrainingOperation(response.Headers.Location, ServiceClient, Diagnostics); } #endregion diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs index ce275c0485857..b14e3e7fb73a8 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs @@ -20,6 +20,9 @@ public class RecognizeContentOperation : Operation /// Provides communication with the Form Recognizer Azure Cognitive Service through its REST API. private readonly ServiceRestClient _serviceClient; + /// Provides tools for exception creation in case of failure. + private readonly ClientDiagnostics _diagnostics; + /// The last HTTP response received from the server. null until the first response is received. private Response _response; @@ -33,7 +36,18 @@ public class RecognizeContentOperation : Operation public override string Id { get; } /// - public override FormPageCollection Value => OperationHelpers.GetValue(ref _value); + public override FormPageCollection Value + { + get + { + if (HasCompleted && !HasValue) +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + throw new InvalidOperationException("RecognizeContent operation failed."); +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + else + return OperationHelpers.GetValue(ref _value); + } + } /// public override bool HasCompleted => _hasCompleted; @@ -52,16 +66,19 @@ public RecognizeContentOperation(string operationId, FormRecognizerClient client Id = operationId; _serviceClient = client.ServiceClient; + _diagnostics = client.Diagnostics; } /// /// Initializes a new instance of the class. /// /// The client for communicating with the Form Recognizer Azure Cognitive Service through its REST API. + /// The client diagnostics for exception creation in case of failure. /// The address of the long-running operation. It can be obtained from the response headers upon starting the operation. - internal RecognizeContentOperation(ServiceRestClient serviceClient, string operationLocation) + internal RecognizeContentOperation(ServiceRestClient serviceClient, ClientDiagnostics diagnostics, string operationLocation) { _serviceClient = serviceClient; + _diagnostics = diagnostics; // TODO: Add validation here // https://github.com/Azure/azure-sdk-for-net/issues/10385 @@ -101,14 +118,20 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke ? await _serviceClient.GetAnalyzeLayoutResultAsync(new Guid(Id), cancellationToken).ConfigureAwait(false) : _serviceClient.GetAnalyzeLayoutResult(new Guid(Id), cancellationToken); - if (update.Value.Status == OperationStatus.Succeeded || update.Value.Status == OperationStatus.Failed) + _response = update.GetRawResponse(); + + if (update.Value.Status == OperationStatus.Succeeded) { _hasCompleted = true; _value = ConvertValue(update.Value.AnalyzeResult.PageResults, update.Value.AnalyzeResult.ReadResults); } + else if (update.Value.Status == OperationStatus.Failed) + { + _hasCompleted = true; - _response = update.GetRawResponse(); + throw await CreateExceptionForFailedOperationAsync(async, update.Value.AnalyzeResult.Errors).ConfigureAwait(false); + } } return GetRawResponse(); @@ -128,5 +151,36 @@ private static FormPageCollection ConvertValue(IReadOnlyList CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) + { + string errorMessage = default; + string errorCode = default; + + if (errors.Count > 0) + { + var firstError = errors[0]; + + errorMessage = firstError.Message; + errorCode = firstError.ErrorCode; + } + else + { + errorMessage = "RecognizeContent operation failed."; + } + + var errorInfo = new Dictionary(); + int index = 0; + + foreach (var error in errors) + { + errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); + index++; + } + + return async + ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) + : _diagnostics.CreateRequestFailedException(_response, errorMessage, errorCode, errorInfo); + } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs index 5a30036618476..412243493ce4d 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs @@ -42,7 +42,18 @@ public class RecognizeCustomFormsOperation : Operation public override string Id { get; } /// - public override RecognizedFormCollection Value => OperationHelpers.GetValue(ref _value); + public override RecognizedFormCollection Value + { + get + { + if (HasCompleted && !HasValue) +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + throw new InvalidOperationException("RecognizeCustomForms operation failed."); +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + else + return OperationHelpers.GetValue(ref _value); + } + } /// public override bool HasCompleted => _hasCompleted; @@ -148,7 +159,6 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke else if (update.Value.Status == OperationStatus.Failed) { _hasCompleted = true; - _value = new RecognizedFormCollection(new List()); throw await CreateExceptionForFailedOperationAsync(async, update.Value.AnalyzeResult.Errors).ConfigureAwait(false); } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs index 7b14c15f8c894..46e7cec0cd6cd 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs @@ -19,6 +19,9 @@ public class RecognizeReceiptsOperation : Operation /// Provides communication with the Form Recognizer Azure Cognitive Service through its REST API. private readonly ServiceRestClient _serviceClient; + /// Provides tools for exception creation in case of failure. + private readonly ClientDiagnostics _diagnostics; + /// The last HTTP response received from the server. null until the first response is received. private Response _response; @@ -32,7 +35,18 @@ public class RecognizeReceiptsOperation : Operation public override string Id { get; } /// - public override RecognizedReceiptCollection Value => OperationHelpers.GetValue(ref _value); + public override RecognizedReceiptCollection Value + { + get + { + if (HasCompleted && !HasValue) +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + throw new InvalidOperationException("RecognizeReceipt operation failed."); +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + else + return OperationHelpers.GetValue(ref _value); + } + } /// public override bool HasCompleted => _hasCompleted; @@ -48,22 +62,25 @@ public class RecognizeReceiptsOperation : Operation /// /// The ID of this operation. /// The client used to check for completion. - public RecognizeReceiptsOperation(string operationId, FormRecognizerClient client) + public RecognizeReceiptsOperation(string operationId,FormRecognizerClient client) { // TODO: Add argument validation here. Id = operationId; _serviceClient = client.ServiceClient; + _diagnostics = client.Diagnostics; } /// /// Initializes a new instance of the class. /// /// The client for communicating with the Form Recognizer Azure Cognitive Service through its REST API. + /// The client diagnostics for exception creation in case of failure. /// The address of the long-running operation. It can be obtained from the response headers upon starting the operation. - internal RecognizeReceiptsOperation(ServiceRestClient serviceClient, string operationLocation) + internal RecognizeReceiptsOperation(ServiceRestClient serviceClient, ClientDiagnostics diagnostics, string operationLocation) { _serviceClient = serviceClient; + _diagnostics = diagnostics; // TODO: Add validation here // https://github.com/Azure/azure-sdk-for-net/issues/10385 @@ -100,7 +117,9 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke ? await _serviceClient.GetAnalyzeReceiptResultAsync(new Guid(Id), cancellationToken).ConfigureAwait(false) : _serviceClient.GetAnalyzeReceiptResult(new Guid(Id), cancellationToken); - if (update.Value.Status == OperationStatus.Succeeded || update.Value.Status == OperationStatus.Failed) + _response = update.GetRawResponse(); + + if (update.Value.Status == OperationStatus.Succeeded) { _hasCompleted = true; @@ -110,8 +129,12 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke //_value = ConvertToRecognizedReceipts(update.Value.AnalyzeResult.DocumentResults.ToList(), update.Value.AnalyzeResult.ReadResults.ToList()); _value = ConvertToRecognizedReceipts(update.Value.AnalyzeResult); } + else if (update.Value.Status == OperationStatus.Failed) + { + _hasCompleted = true; - _response = update.GetRawResponse(); + throw await CreateExceptionForFailedOperationAsync(async, update.Value.AnalyzeResult.Errors).ConfigureAwait(false); + } } return GetRawResponse(); @@ -126,5 +149,36 @@ private static RecognizedReceiptCollection ConvertToRecognizedReceipts(AnalyzeRe } return new RecognizedReceiptCollection(receipts); } + + private async ValueTask CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) + { + string errorMessage = default; + string errorCode = default; + + if (errors.Count > 0) + { + var firstError = errors[0]; + + errorMessage = firstError.Message; + errorCode = firstError.ErrorCode; + } + else + { + errorMessage = "RecognizeReceipt operation failed."; + } + + var errorInfo = new Dictionary(); + int index = 0; + + foreach (var error in errors) + { + errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); + index++; + } + + return async + ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) + : _diagnostics.CreateRequestFailedException(_response, errorMessage, errorCode, errorInfo); + } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs index b3ed982221709..0c2eabfa680c5 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs @@ -2,9 +2,11 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; +using Azure.AI.FormRecognizer.Models; using Azure.Core; using Azure.Core.Pipeline; @@ -18,6 +20,12 @@ public class TrainingOperation : Operation /// Provides communication with the Form Recognizer Azure Cognitive Service through its REST API. private readonly ServiceRestClient _serviceClient; + /// Provides tools for exception creation in case of failure. + private readonly ClientDiagnostics _diagnostics; + + /// The id of the model created. + private Guid _modelId; + /// The last HTTP response received from the server. null until the first response is received. private Response _response; @@ -31,16 +39,24 @@ public class TrainingOperation : Operation public override string Id { get; } /// - public override CustomFormModel Value => OperationHelpers.GetValue(ref _value); + public override CustomFormModel Value + { + get + { + if (HasCompleted && !HasValue) +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + throw new InvalidOperationException($"Invalid model created with ID {_modelId}."); +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + else + return OperationHelpers.GetValue(ref _value); + } + } /// public override bool HasCompleted => _hasCompleted; /// public override bool HasValue => _value != null; - // TODO: This will make the model available even if status is failed to train. - // is it useful to make the value available if training has failed? - // https://github.com/Azure/azure-sdk-for-net/issues/10392 /// public override Response GetRawResponse() => _response; @@ -53,9 +69,10 @@ public override ValueTask> WaitForCompletionAsync(Canc public override ValueTask> WaitForCompletionAsync(TimeSpan pollingInterval, CancellationToken cancellationToken = default) => this.DefaultWaitForCompletionAsync(pollingInterval, cancellationToken); - internal TrainingOperation(string location, ServiceRestClient allOperations) + internal TrainingOperation(string location, ServiceRestClient allOperations, ClientDiagnostics diagnostics) { _serviceClient = allOperations; + _diagnostics = diagnostics; // TODO: validate this // https://github.com/Azure/azure-sdk-for-net/issues/10385 @@ -70,6 +87,7 @@ internal TrainingOperation(string location, ServiceRestClient allOperations) public TrainingOperation(string operationId, FormTrainingClient client) { Id = operationId; + _diagnostics = client.Diagnostics; _serviceClient = client.ServiceClient; } @@ -96,16 +114,41 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke ? await _serviceClient.GetCustomModelAsync(new Guid(Id), includeKeys: true, cancellationToken).ConfigureAwait(false) : _serviceClient.GetCustomModel(new Guid(Id), includeKeys: true, cancellationToken); - if (update.Value.ModelInfo.Status != CustomFormModelStatus.Training) + _response = update.GetRawResponse(); + + if (update.Value.ModelInfo.Status == CustomFormModelStatus.Ready) { _hasCompleted = true; _value = new CustomFormModel(update.Value); } + else if (update.Value.ModelInfo.Status == CustomFormModelStatus.Invalid) + { + _hasCompleted = true; + _modelId = update.Value.ModelInfo.ModelId; - _response = update.GetRawResponse(); + throw await CreateExceptionForFailedOperationAsync(async, update.Value.TrainResult.Errors).ConfigureAwait(false); + } } return GetRawResponse(); } + + private async ValueTask CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) + { + string errorMessage = $"Invalid model created with ID {_modelId}"; + + var errorInfo = new Dictionary(); + int index = 0; + + foreach (var error in errors) + { + errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); + index++; + } + + return async + ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, additionalInfo:errorInfo).ConfigureAwait(false) + : _diagnostics.CreateRequestFailedException(_response, errorMessage, additionalInfo:errorInfo); + } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs index 5326d0ce1edd6..8d46dc5f49192 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs @@ -102,7 +102,6 @@ public async Task StartRecognizeContentPopulatesFormPage(bool useStream) } await operation.WaitForCompletionAsync(); - Assert.IsTrue(operation.HasValue); var formPage = operation.Value.Single(); @@ -866,8 +865,8 @@ public async Task StartRecognizeCustomFormsFromUriThrowsForNonExistingContent(bo Assert.AreEqual(expectedErrorCode, capturedException.ErrorCode); Assert.True(operation.HasCompleted); - Assert.True(operation.HasValue); - Assert.AreEqual(0, operation.Value.Count); + Assert.False(operation.HasValue); + Assert.Throws(() => operation.Value.GetType()); } private void ValidateFormPage(FormPage formPage, bool includeTextContent, int expectedPageNumber) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs index f8831afc602b0..3376cb9482473 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs @@ -109,22 +109,10 @@ public async Task StartTrainingError() var containerUrl = new Uri("https://someUrl"); TrainingOperation operation = await client.StartTrainingAsync(containerUrl, useTrainingLabels: false); + Assert.ThrowsAsync(async () => await operation.WaitForCompletionAsync()); - await operation.WaitForCompletionAsync(); - - Assert.IsTrue(operation.HasValue); - - CustomFormModel model = operation.Value; - - Assert.IsNotNull(model.ModelId); - Assert.IsNotNull(model.RequestedOn); - Assert.IsNotNull(model.CompletedOn); - Assert.IsNotNull(model.Status); - Assert.AreEqual(CustomFormModelStatus.Invalid, model.Status); - Assert.IsNotNull(model.Errors); - Assert.AreEqual(1, model.Errors.Count); - Assert.IsNotNull(model.Errors.FirstOrDefault().ErrorCode); - Assert.IsNotNull(model.Errors.FirstOrDefault().Message); + Assert.False(operation.HasValue); + Assert.Throws(() => operation.Value.GetType()); } [Test] From d1ff8c53f100ec312663e6e6564837e64754642a Mon Sep 17 00:00:00 2001 From: Mariana Rios Flores Date: Tue, 2 Jun 2020 23:52:59 -0700 Subject: [PATCH 2/5] changelog --- sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md b/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md index 58a3a355178a6..2acc2dce793cc 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md @@ -30,6 +30,10 @@ - Protected constructors have been removed from `Operation` types, such as `TrainingOperation` or `RecognizeContentOperation`. - `USReceipt`, `USReceiptItem`, `USReceiptType` and `FormField{T}` types removed. Information about a `RecognizedReceipt` must now be extracted from its `RecognizedForm`. - `ReceiptLocale` removed from `RecognizedReceipt`. +- An `InvalidOperationException` is now raised if trying to access the `Value` property of a `TrainingOperation` when a trained model is invalid. +- A `RequestFailedException` is now raised if a model with `status=="invalid"` is returned from the `StartTraining` and `StartTrainingAsync` methods. +- A `RequestFailedException` is now raised if an operation like `StartRecognizeReceipts`, `StartRecognizeContent`, or `StartRecognizeCustomForms` fails. +- An `InvalidOperationException` is now raised if trying to access the `Value` property of a `xxOperation` object when a the executed operation failed. ### New Features From beaf45d009e74642c3a6ec0d59ad72fb059b8c95 Mon Sep 17 00:00:00 2001 From: Mariana Rios Flores Date: Thu, 4 Jun 2020 16:39:40 -0700 Subject: [PATCH 3/5] PR Feedback --- .../Azure.AI.FormRecognizer/CHANGELOG.md | 2 +- .../src/ClientCommon.cs | 37 ++++++++++++++ .../src/CopyModelOperation.cs | 44 ++++------------ .../src/FormTrainingClient.cs | 6 ++- .../src/RecognizeContentOperation.cs | 45 ++++------------ .../src/RecognizeCustomFormsOperation.cs | 44 ++++------------ .../src/RecognizeReceiptsOperation.cs | 51 ++++--------------- .../src/TrainingOperation.cs | 36 ++++--------- .../FormRecognizerClientLiveTests.cs | 2 +- .../FormTrainingClientLiveTests.cs | 2 +- 10 files changed, 92 insertions(+), 177 deletions(-) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md b/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md index 2acc2dce793cc..43f1e3eedac47 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md @@ -32,7 +32,7 @@ - `ReceiptLocale` removed from `RecognizedReceipt`. - An `InvalidOperationException` is now raised if trying to access the `Value` property of a `TrainingOperation` when a trained model is invalid. - A `RequestFailedException` is now raised if a model with `status=="invalid"` is returned from the `StartTraining` and `StartTrainingAsync` methods. -- A `RequestFailedException` is now raised if an operation like `StartRecognizeReceipts`, `StartRecognizeContent`, or `StartRecognizeCustomForms` fails. +- A `RequestFailedException` is now raised if an operation like `StartRecognizeReceipts` or `StartRecognizeContent` fails. - An `InvalidOperationException` is now raised if trying to access the `Value` property of a `xxOperation` object when a the executed operation failed. ### New Features diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/ClientCommon.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/ClientCommon.cs index 3fce1528197e0..7762b9952fc73 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/ClientCommon.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/ClientCommon.cs @@ -2,6 +2,10 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Azure.AI.FormRecognizer.Models; +using Azure.Core.Pipeline; namespace Azure.AI.FormRecognizer { @@ -30,5 +34,38 @@ public static Guid ValidateModelId(string modelId, string paramName) return guid; } + + public static async ValueTask CreateExceptionForFailedOperationAsync(bool async, ClientDiagnostics diagnostics, Response response, IReadOnlyList errors, string errorMessage = default) + { + string errorCode = default; + + if (string.IsNullOrEmpty(errorMessage)) + { + if (errors.Count > 0) + { + var firstError = errors[0]; + + errorMessage = firstError.Message; + errorCode = firstError.ErrorCode; + } + else + { + errorMessage = "Operation failed."; + } + } + + var errorInfo = new Dictionary(); + int index = 0; + + foreach (var error in errors) + { + errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); + index++; + } + + return async + ? await diagnostics.CreateRequestFailedExceptionAsync(response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) + : diagnostics.CreateRequestFailedException(response, errorMessage, errorCode, errorInfo); + } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs index 1e2e631ece7a1..934e685ed7c79 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs @@ -41,6 +41,8 @@ public class CopyModelOperation : Operation /// An ID representing the operation that can be used along with to poll for the status of the long-running operation. private readonly string _resultId; + private RequestFailedException _requestFailedException; + /// public override string Id { get; } @@ -51,7 +53,7 @@ public override CustomFormModelInfo Value { if (HasCompleted && !HasValue) #pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new InvalidOperationException("Copy model operation failed"); + throw _requestFailedException; #pragma warning restore CA1065 // Do not raise exceptions in unexpected locations else return OperationHelpers.GetValue(ref _value); @@ -164,14 +166,17 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - _hasCompleted = true; + // we need to first assign a vaue and then mark the operation as completed to avoid race conditions _value = ConvertValue(update.Value, _targetModelId, CustomFormModelStatus.Ready); + _hasCompleted = true; } else if (update.Value.Status == OperationStatus.Failed) { + _requestFailedException = await ClientCommon + .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.CopyResult.Errors) + .ConfigureAwait(false); _hasCompleted = true; - - throw await CreateExceptionForFailedOperationAsync(async, update.Value.CopyResult.Errors).ConfigureAwait(false); + throw _requestFailedException; } } @@ -186,36 +191,5 @@ private static CustomFormModelInfo ConvertValue(CopyOperationResult result, stri result.LastUpdatedDateTime, status); } - - private async ValueTask CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) - { - string errorMessage = default; - string errorCode = default; - - if (errors.Count > 0) - { - var firstError = errors[0]; - - errorMessage = firstError.Message; - errorCode = firstError.ErrorCode; - } - else - { - errorMessage = "Copy model operation failed."; - } - - var errorInfo = new Dictionary(); - int index = 0; - - foreach (var error in errors) - { - errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); - index++; - } - - return async - ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) - : _diagnostics.CreateRequestFailedException(_response, errorMessage, errorCode, errorInfo); - } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs index 8fdb4a5c38021..9696d20cbe588 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs @@ -114,7 +114,8 @@ public FormTrainingClient(Uri endpoint, TokenCredential credential, FormRecogniz /// /// A to wait on this long-running operation. Its .Value upon successful /// completion will contain meta-data about the trained model. - /// If the training fails, an exception is raised, and a model with an "invalid" status is still created in the Form Recognizer resource. + /// Even if training fails, a model is created in the Form Recognizer account with an "invalid" status. + /// A will be raised containing the modelId to access this invalid model. /// [ForwardsClientCalls] public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTrainingLabels, TrainingFileFilter trainingFileFilter = default, CancellationToken cancellationToken = default) @@ -137,7 +138,8 @@ public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTra /// /// A to wait on this long-running operation. Its .Value upon successful /// completion will contain meta-data about the trained model. - /// If the training fails, an exception is raised, and a model with an "invalid" status is still created in the Form Recognizer resource. + /// Even if training fails, a model is created in the Form Recognizer account with an "invalid" status. + /// A will be raised containing the modelId to access this invalid model. /// [ForwardsClientCalls] public virtual async Task StartTrainingAsync(Uri trainingFilesUri, bool useTrainingLabels, TrainingFileFilter trainingFileFilter = default, CancellationToken cancellationToken = default) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs index b14e3e7fb73a8..6d84af9b1f1b9 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeContentOperation.cs @@ -23,6 +23,8 @@ public class RecognizeContentOperation : Operation /// Provides tools for exception creation in case of failure. private readonly ClientDiagnostics _diagnostics; + private RequestFailedException _requestFailedException; + /// The last HTTP response received from the server. null until the first response is received. private Response _response; @@ -42,7 +44,7 @@ public override FormPageCollection Value { if (HasCompleted && !HasValue) #pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new InvalidOperationException("RecognizeContent operation failed."); + throw _requestFailedException; #pragma warning restore CA1065 // Do not raise exceptions in unexpected locations else return OperationHelpers.GetValue(ref _value); @@ -122,15 +124,17 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - _hasCompleted = true; - + // we need to first assign a vaue and then mark the operation as completed to avoid race conditions _value = ConvertValue(update.Value.AnalyzeResult.PageResults, update.Value.AnalyzeResult.ReadResults); + _hasCompleted = true; } else if (update.Value.Status == OperationStatus.Failed) { + _requestFailedException = await ClientCommon + .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.AnalyzeResult.Errors) + .ConfigureAwait(false); _hasCompleted = true; - - throw await CreateExceptionForFailedOperationAsync(async, update.Value.AnalyzeResult.Errors).ConfigureAwait(false); + throw _requestFailedException; } } @@ -151,36 +155,5 @@ private static FormPageCollection ConvertValue(IReadOnlyList CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) - { - string errorMessage = default; - string errorCode = default; - - if (errors.Count > 0) - { - var firstError = errors[0]; - - errorMessage = firstError.Message; - errorCode = firstError.ErrorCode; - } - else - { - errorMessage = "RecognizeContent operation failed."; - } - - var errorInfo = new Dictionary(); - int index = 0; - - foreach (var error in errors) - { - errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); - index++; - } - - return async - ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) - : _diagnostics.CreateRequestFailedException(_response, errorMessage, errorCode, errorInfo); - } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs index 412243493ce4d..44eaebf1ee19d 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs @@ -38,6 +38,8 @@ public class RecognizeCustomFormsOperation : Operation /// An ID representing the operation that can be used along with to poll for the status of the long-running operation. private readonly string _resultId; + private RequestFailedException _requestFailedException; + /// public override string Id { get; } @@ -48,7 +50,7 @@ public override RecognizedFormCollection Value { if (HasCompleted && !HasValue) #pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new InvalidOperationException("RecognizeCustomForms operation failed."); + throw _requestFailedException; #pragma warning restore CA1065 // Do not raise exceptions in unexpected locations else return OperationHelpers.GetValue(ref _value); @@ -153,14 +155,17 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - _hasCompleted = true; + // we need to first assign a vaue and then mark the operation as completed to avoid race conditions _value = ConvertToRecognizedForms(update.Value.AnalyzeResult); + _hasCompleted = true; } else if (update.Value.Status == OperationStatus.Failed) { + _requestFailedException = await ClientCommon + .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.AnalyzeResult.Errors) + .ConfigureAwait(false); _hasCompleted = true; - - throw await CreateExceptionForFailedOperationAsync(async, update.Value.AnalyzeResult.Errors).ConfigureAwait(false); + throw _requestFailedException; } } @@ -193,36 +198,5 @@ private static RecognizedFormCollection ConvertSupervisedResult(AnalyzeResult_in } return new RecognizedFormCollection(forms); } - - private async ValueTask CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) - { - string errorMessage = default; - string errorCode = default; - - if (errors.Count > 0) - { - var firstError = errors[0]; - - errorMessage = firstError.Message; - errorCode = firstError.ErrorCode; - } - else - { - errorMessage = "RecognizeCustomForms operation failed."; - } - - var errorInfo = new Dictionary(); - int index = 0; - - foreach (var error in errors) - { - errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); - index++; - } - - return async - ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) - : _diagnostics.CreateRequestFailedException(_response, errorMessage, errorCode, errorInfo); - } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs index 46e7cec0cd6cd..a153a09bb5885 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs @@ -22,6 +22,8 @@ public class RecognizeReceiptsOperation : Operation /// Provides tools for exception creation in case of failure. private readonly ClientDiagnostics _diagnostics; + private RequestFailedException _requestFailedException; + /// The last HTTP response received from the server. null until the first response is received. private Response _response; @@ -41,7 +43,7 @@ public override RecognizedReceiptCollection Value { if (HasCompleted && !HasValue) #pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new InvalidOperationException("RecognizeReceipt operation failed."); + throw _requestFailedException; #pragma warning restore CA1065 // Do not raise exceptions in unexpected locations else return OperationHelpers.GetValue(ref _value); @@ -62,7 +64,7 @@ public override RecognizedReceiptCollection Value /// /// The ID of this operation. /// The client used to check for completion. - public RecognizeReceiptsOperation(string operationId,FormRecognizerClient client) + public RecognizeReceiptsOperation(string operationId, FormRecognizerClient client) { // TODO: Add argument validation here. @@ -121,19 +123,17 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - _hasCompleted = true; - - // TODO: When they support extracting more than one receipt, add a pageable method for this. - // https://github.com/Azure/azure-sdk-for-net/issues/10389 - - //_value = ConvertToRecognizedReceipts(update.Value.AnalyzeResult.DocumentResults.ToList(), update.Value.AnalyzeResult.ReadResults.ToList()); + // we need to first assign a vaue and then mark the operation as completed to avoid race conditions _value = ConvertToRecognizedReceipts(update.Value.AnalyzeResult); + _hasCompleted = true; } else if (update.Value.Status == OperationStatus.Failed) { + _requestFailedException = await ClientCommon + .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.AnalyzeResult.Errors) + .ConfigureAwait(false); _hasCompleted = true; - - throw await CreateExceptionForFailedOperationAsync(async, update.Value.AnalyzeResult.Errors).ConfigureAwait(false); + throw _requestFailedException; } } @@ -149,36 +149,5 @@ private static RecognizedReceiptCollection ConvertToRecognizedReceipts(AnalyzeRe } return new RecognizedReceiptCollection(receipts); } - - private async ValueTask CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) - { - string errorMessage = default; - string errorCode = default; - - if (errors.Count > 0) - { - var firstError = errors[0]; - - errorMessage = firstError.Message; - errorCode = firstError.ErrorCode; - } - else - { - errorMessage = "RecognizeReceipt operation failed."; - } - - var errorInfo = new Dictionary(); - int index = 0; - - foreach (var error in errors) - { - errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); - index++; - } - - return async - ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, errorCode, errorInfo).ConfigureAwait(false) - : _diagnostics.CreateRequestFailedException(_response, errorMessage, errorCode, errorInfo); - } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs index 0c2eabfa680c5..34496a464d043 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs @@ -23,8 +23,7 @@ public class TrainingOperation : Operation /// Provides tools for exception creation in case of failure. private readonly ClientDiagnostics _diagnostics; - /// The id of the model created. - private Guid _modelId; + private RequestFailedException _requestFailedException; /// The last HTTP response received from the server. null until the first response is received. private Response _response; @@ -45,7 +44,7 @@ public override CustomFormModel Value { if (HasCompleted && !HasValue) #pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new InvalidOperationException($"Invalid model created with ID {_modelId}."); + throw _requestFailedException; #pragma warning restore CA1065 // Do not raise exceptions in unexpected locations else return OperationHelpers.GetValue(ref _value); @@ -118,37 +117,24 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.ModelInfo.Status == CustomFormModelStatus.Ready) { - _hasCompleted = true; + // we need to first assign a vaue and then mark the operation as completed to avoid race conditions _value = new CustomFormModel(update.Value); + _hasCompleted = true; } else if (update.Value.ModelInfo.Status == CustomFormModelStatus.Invalid) { + _requestFailedException = await ClientCommon.CreateExceptionForFailedOperationAsync( + async, + _diagnostics, + _response, + update.Value.TrainResult.Errors, + $"Invalid model created with ID {update.Value.ModelInfo.ModelId}").ConfigureAwait(false); _hasCompleted = true; - _modelId = update.Value.ModelInfo.ModelId; - - throw await CreateExceptionForFailedOperationAsync(async, update.Value.TrainResult.Errors).ConfigureAwait(false); + throw _requestFailedException; } } return GetRawResponse(); } - - private async ValueTask CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList errors) - { - string errorMessage = $"Invalid model created with ID {_modelId}"; - - var errorInfo = new Dictionary(); - int index = 0; - - foreach (var error in errors) - { - errorInfo.Add($"error-{index}", $"{error.ErrorCode}: {error.Message}"); - index++; - } - - return async - ? await _diagnostics.CreateRequestFailedExceptionAsync(_response, errorMessage, additionalInfo:errorInfo).ConfigureAwait(false) - : _diagnostics.CreateRequestFailedException(_response, errorMessage, additionalInfo:errorInfo); - } } } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs index 8d46dc5f49192..ea07c9a9279df 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormRecognizerClient/FormRecognizerClientLiveTests.cs @@ -866,7 +866,7 @@ public async Task StartRecognizeCustomFormsFromUriThrowsForNonExistingContent(bo Assert.True(operation.HasCompleted); Assert.False(operation.HasValue); - Assert.Throws(() => operation.Value.GetType()); + Assert.Throws(() => operation.Value.GetType()); } private void ValidateFormPage(FormPage formPage, bool includeTextContent, int expectedPageNumber) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs index 3376cb9482473..4922a1e89498d 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/tests/FormTrainingClient/FormTrainingClientLiveTests.cs @@ -112,7 +112,7 @@ public async Task StartTrainingError() Assert.ThrowsAsync(async () => await operation.WaitForCompletionAsync()); Assert.False(operation.HasValue); - Assert.Throws(() => operation.Value.GetType()); + Assert.Throws(() => operation.Value.GetType()); } [Test] From 8e80b6e3e01b697cde03574faf49468c44725b0d Mon Sep 17 00:00:00 2001 From: Mariana Rios Flores Date: Thu, 4 Jun 2020 20:41:16 -0700 Subject: [PATCH 4/5] don't overwrite history.... --- sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md b/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md index 43f1e3eedac47..12d4ee4f2ca28 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/CHANGELOG.md @@ -33,7 +33,7 @@ - An `InvalidOperationException` is now raised if trying to access the `Value` property of a `TrainingOperation` when a trained model is invalid. - A `RequestFailedException` is now raised if a model with `status=="invalid"` is returned from the `StartTraining` and `StartTrainingAsync` methods. - A `RequestFailedException` is now raised if an operation like `StartRecognizeReceipts` or `StartRecognizeContent` fails. -- An `InvalidOperationException` is now raised if trying to access the `Value` property of a `xxOperation` object when a the executed operation failed. +- An `InvalidOperationException` is now raised if trying to access the `Value` property of a `xxOperation` object when the executed operation failed. ### New Features From fc1806fbc5925361ac5d94f89f2bcfa268588c6b Mon Sep 17 00:00:00 2001 From: Mariana Rios Flores Date: Fri, 5 Jun 2020 14:34:14 -0700 Subject: [PATCH 5/5] improve race condition comment --- .../Azure.AI.FormRecognizer/src/CopyModelOperation.cs | 2 +- .../Azure.AI.FormRecognizer/src/FormRecognizerClient.cs | 2 +- .../src/RecognizeCustomFormsOperation.cs | 2 +- .../Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs | 2 +- .../Azure.AI.FormRecognizer/src/TrainingOperation.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs index 934e685ed7c79..a16a4309b45e3 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs @@ -166,7 +166,7 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - // we need to first assign a vaue and then mark the operation as completed to avoid race conditions + // We need to first assign a value and then mark the operation as completed to avoid a race condition with the getter in Value _value = ConvertValue(update.Value, _targetModelId, CustomFormModelStatus.Ready); _hasCompleted = true; } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs index 290fa9e24e637..0a24d88bdd6c9 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormRecognizerClient.cs @@ -133,7 +133,7 @@ public virtual RecognizeContentOperation StartRecognizeContent(Stream form, Reco FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(form, nameof(form)); ResponseWithHeaders response = ServiceClient.AnalyzeLayoutAsync(contentType, form, cancellationToken); - return new RecognizeContentOperation(ServiceClient, Diagnostics,response.Headers.OperationLocation); + return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation); } /// diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs index 44eaebf1ee19d..18e73c2fce3d3 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeCustomFormsOperation.cs @@ -155,7 +155,7 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - // we need to first assign a vaue and then mark the operation as completed to avoid race conditions + // We need to first assign a value and then mark the operation as completed to avoid a race condition with the getter in Value _value = ConvertToRecognizedForms(update.Value.AnalyzeResult); _hasCompleted = true; } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs index a153a09bb5885..f1579b407465c 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/RecognizeReceiptsOperation.cs @@ -123,7 +123,7 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.Status == OperationStatus.Succeeded) { - // we need to first assign a vaue and then mark the operation as completed to avoid race conditions + // We need to first assign a value and then mark the operation as completed to avoid a race condition with the getter in Value _value = ConvertToRecognizedReceipts(update.Value.AnalyzeResult); _hasCompleted = true; } diff --git a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs index 34496a464d043..142fbad17c1f5 100644 --- a/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs +++ b/sdk/formrecognizer/Azure.AI.FormRecognizer/src/TrainingOperation.cs @@ -117,7 +117,7 @@ private async ValueTask UpdateStatusAsync(bool async, CancellationToke if (update.Value.ModelInfo.Status == CustomFormModelStatus.Ready) { - // we need to first assign a vaue and then mark the operation as completed to avoid race conditions + // We need to first assign a value and then mark the operation as completed to avoid a race condition with the getter in Value _value = new CustomFormModel(update.Value); _hasCompleted = true; }