Consistency operation failed status management in LRO

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` or `StartRecognizeContent` fails.
+- An `InvalidOperationException` is now raised if trying to access the `Value` property of a `xxOperation` object when the executed operation failed.
 
 ### New Features
 

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<RequestFailedException> CreateExceptionForFailedOperationAsync(bool async, ClientDiagnostics diagnostics, Response response, IReadOnlyList<FormRecognizerError> 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<string, string>();
+            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);
+        }
     }
 }

sdk/formrecognizer/Azure.AI.FormRecognizer/src/CopyModelOperation.cs

@@ -41,11 +41,24 @@ public class CopyModelOperation : Operation<CustomFormModelInfo>
         /// <summary>An ID representing the operation that can be used along with <see cref="_modelId"/> to poll for the status of the long-running operation.</summary>
         private readonly string _resultId;
 
+        private RequestFailedException _requestFailedException;
+
         /// <inheritdoc/>
         public override string Id { get; }
 
         /// <inheritdoc/>
-        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 _requestFailedException;
+#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations
+                else
+                    return OperationHelpers.GetValue(ref _value);
+            }
+        }
 
         /// <inheritdoc/>
         public override bool HasCompleted => _hasCompleted;
@@ -88,7 +101,7 @@ public CopyModelOperation(string operationId, string targetModelId, FormTraining
         /// Initializes a new instance of the <see cref="CopyModelOperation"/> class.
         /// </summary>
         /// <param name="serviceClient">The client for communicating with the Form Recognizer Azure Cognitive Service through its REST API.</param>
-        /// <param name="diagnostics">Provides tools for exception creation in case of failure.</param>
+        /// <param name="diagnostics">The client diagnostics for exception creation in case of failure.</param>
         /// <param name="operationLocation">The address of the long-running operation. It can be obtained from the response headers upon starting the operation.</param>
         /// <param name="targetModelId">Model id in the target Form Recognizer Resource.</param>
         internal CopyModelOperation(ServiceRestClient serviceClient, ClientDiagnostics diagnostics, string operationLocation, string targetModelId)
@@ -153,14 +166,17 @@ private async ValueTask<Response> UpdateStatusAsync(bool async, CancellationToke
 
                 if (update.Value.Status == OperationStatus.Succeeded)
                 {
-                    _hasCompleted = true;
+                    // 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;
                 }
                 else if (update.Value.Status == OperationStatus.Failed)
                 {
+                    _requestFailedException = await ClientCommon
+                        .CreateExceptionForFailedOperationAsync(async, _diagnostics, _response, update.Value.CopyResult.Errors)
+                        .ConfigureAwait(false);
                     _hasCompleted = true;
-                    _value = ConvertValue(update.Value, _targetModelId, CustomFormModelStatus.Invalid);
-                    throw await CreateExceptionForFailedOperationAsync(async, update.Value.CopyResult.Errors).ConfigureAwait(false);
+                    throw _requestFailedException;
                 }
             }
 
@@ -175,36 +191,5 @@ private static CustomFormModelInfo ConvertValue(CopyOperationResult result, stri
                 result.LastUpdatedDateTime,
                 status);
         }
-
-        private async ValueTask<RequestFailedException> CreateExceptionForFailedOperationAsync(bool async, IReadOnlyList<FormRecognizerError> 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<string, string>();
-            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);
-        }
     }
 }

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<ServiceAnalyzeLayoutAsyncHeaders> response =  ServiceClient.AnalyzeLayoutAsync(contentType, form, cancellationToken);
-            return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation);
+            return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation);
         }
 
         /// <summary>
@@ -153,7 +153,7 @@ public virtual async Task<RecognizeContentOperation> StartRecognizeContentAsync(
             FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(form, nameof(form));
 
             ResponseWithHeaders<ServiceAnalyzeLayoutAsyncHeaders> response = await ServiceClient.AnalyzeLayoutAsyncAsync(contentType, form, cancellationToken).ConfigureAwait(false);
-            return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation);
+            return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation);
         }
 
         /// <summary>
@@ -171,7 +171,7 @@ public virtual RecognizeContentOperation StartRecognizeContentFromUri(Uri formUr
 
             SourcePath_internal sourcePath = new SourcePath_internal(formUrl.AbsoluteUri);
             ResponseWithHeaders<ServiceAnalyzeLayoutAsyncHeaders> response = ServiceClient.AnalyzeLayoutAsync(sourcePath, cancellationToken);
-            return new RecognizeContentOperation(ServiceClient, response.Headers.OperationLocation);
+            return new RecognizeContentOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation);
         }
 
         /// <summary>
@@ -189,7 +189,7 @@ public virtual async Task<RecognizeContentOperation> StartRecognizeContentFromUr
 
             SourcePath_internal sourcePath = new SourcePath_internal(formUrl.AbsoluteUri);
             ResponseWithHeaders<ServiceAnalyzeLayoutAsyncHeaders> 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<RecognizeReceiptsOperation> StartRecognizeReceiptsAsyn
             FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(receipt, nameof(receipt));
 
             ResponseWithHeaders<ServiceAnalyzeReceiptAsyncHeaders> 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);
         }
 
         /// <summary>
@@ -233,7 +233,7 @@ public virtual RecognizeReceiptsOperation StartRecognizeReceipts(Stream receipt,
             FormContentType contentType = recognizeOptions.ContentType ?? DetectContentType(receipt, nameof(receipt));
 
             ResponseWithHeaders<ServiceAnalyzeReceiptAsyncHeaders> response = ServiceClient.AnalyzeReceiptAsync(contentType, receipt, includeTextDetails: recognizeOptions.IncludeTextContent, cancellationToken);
-            return new RecognizeReceiptsOperation(ServiceClient, response.Headers.OperationLocation);
+            return new RecognizeReceiptsOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation);
         }
 
         /// <summary>
@@ -253,7 +253,7 @@ public virtual async Task<RecognizeReceiptsOperation> StartRecognizeReceiptsFrom
 
             SourcePath_internal sourcePath = new SourcePath_internal(receiptUrl.AbsoluteUri);
             ResponseWithHeaders<ServiceAnalyzeReceiptAsyncHeaders> 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);
         }
 
         /// <summary>
@@ -273,7 +273,7 @@ public virtual RecognizeReceiptsOperation StartRecognizeReceiptsFromUri(Uri rece
 
             SourcePath_internal sourcePath = new SourcePath_internal(receiptUrl.AbsoluteUri);
             ResponseWithHeaders<ServiceAnalyzeReceiptAsyncHeaders> response = ServiceClient.AnalyzeReceiptAsync(includeTextDetails: recognizeOptions.IncludeTextContent, sourcePath, cancellationToken);
-            return new RecognizeReceiptsOperation(ServiceClient, response.Headers.OperationLocation);
+            return new RecognizeReceiptsOperation(ServiceClient, Diagnostics, response.Headers.OperationLocation);
         }
 
         #endregion

sdk/formrecognizer/Azure.AI.FormRecognizer/src/FormTrainingClient.cs

@@ -111,8 +111,12 @@ public FormTrainingClient(Uri endpoint, TokenCredential credential, FormRecogniz
         /// <param name="useTrainingLabels">If <c>true</c>, use a label file created in the &lt;link-to-label-tool-doc&gt; to provide training-time labels for training a model. If <c>false</c>, the model will be trained from forms only.</param>
         /// <param name="trainingFileFilter">Filter to apply to the documents in the source path for training.</param>
         /// <param name="cancellationToken">A <see cref="CancellationToken"/> controlling the request lifetime.</param>
-        /// <returns>A <see cref="TrainingOperation"/> to wait on this long-running operation.  Its <see cref="TrainingOperation"/>.Value upon successful
-        /// completion will contain meta-data about the trained model.</returns>
+        /// <returns>
+        /// <para>A <see cref="TrainingOperation"/> to wait on this long-running operation. Its <see cref="TrainingOperation"/>.Value upon successful
+        /// completion will contain meta-data about the trained model.</para>
+        /// <para>Even if training fails, a model is created in the Form Recognizer account with an "invalid" status.
+        /// A <see cref="RequestFailedException"/> will be raised containing the modelId to access this invalid model.</para>
+        /// </returns>
         [ForwardsClientCalls]
         public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTrainingLabels, TrainingFileFilter trainingFileFilter = default, CancellationToken cancellationToken = default)
         {
@@ -121,7 +125,7 @@ public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTra
             var trainRequest = new TrainRequest_internal(trainingFilesUri.AbsoluteUri, trainingFileFilter, useTrainingLabels);
 
             ResponseWithHeaders<ServiceTrainCustomModelAsyncHeaders> response = ServiceClient.TrainCustomModelAsync(trainRequest);
-            return new TrainingOperation(response.Headers.Location, ServiceClient);
+            return new TrainingOperation(response.Headers.Location, ServiceClient, Diagnostics);
         }
 
         /// <summary>
@@ -131,8 +135,12 @@ public virtual TrainingOperation StartTraining(Uri trainingFilesUri, bool useTra
         /// <param name="useTrainingLabels">If <c>true</c>, use a label file created in the &lt;link-to-label-tool-doc&gt; to provide training-time labels for training a model. If <c>false</c>, the model will be trained from forms only.</param>
         /// <param name="trainingFileFilter">Filter to apply to the documents in the source path for training.</param>
         /// <param name="cancellationToken">A <see cref="CancellationToken"/> controlling the request lifetime.</param>
-        /// <returns>A <see cref="TrainingOperation"/> to wait on this long-running operation.  Its <see cref="TrainingOperation"/>.Value upon successful
-        /// completion will contain meta-data about the trained model.</returns>
+        /// <returns>
+        /// <para>A <see cref="TrainingOperation"/> to wait on this long-running operation. Its <see cref="TrainingOperation"/>.Value upon successful
+        /// completion will contain meta-data about the trained model.</para>
+        /// <para>Even if training fails, a model is created in the Form Recognizer account with an "invalid" status.
+        /// A <see cref="RequestFailedException"/> will be raised containing the modelId to access this invalid model.</para>
+        /// </returns>
         [ForwardsClientCalls]
         public virtual async Task<TrainingOperation> StartTrainingAsync(Uri trainingFilesUri, bool useTrainingLabels, TrainingFileFilter trainingFileFilter = default, CancellationToken cancellationToken = default)
         {
@@ -141,7 +149,7 @@ public virtual async Task<TrainingOperation> StartTrainingAsync(Uri trainingFile
             var trainRequest = new TrainRequest_internal(trainingFilesUri.AbsoluteUri, trainingFileFilter, useTrainingLabels);
 
             ResponseWithHeaders<ServiceTrainCustomModelAsyncHeaders> response = await ServiceClient.TrainCustomModelAsyncAsync(trainRequest).ConfigureAwait(false);
-            return new TrainingOperation(response.Headers.Location, ServiceClient);
+            return new TrainingOperation(response.Headers.Location, ServiceClient, Diagnostics);
         }
 
         #endregion