.NET LRO error handling is inconsistent with guidelines, each other

[heaths]

While fixing(?) issue #41855 where we weren't throwing from an LRO for a failed status because the status monitor endpoint didn't return 200, I found that a lot of our hand-authored operations, our generation 1 (track 2) generated operations, and our generation 2 generated operations all seem to work a little differently and they don't seem to always follow our guidelines, which currently state:

✅ DO throw from methods on Operation subclasses in the following scenarios.

• If an underlying service operation call from UpdateStatus, WaitForCompletion, or WaitForCompletionAsync throws, re-throw RequestFailedException or its subtype.
• If the operation completes with a non-success result, throw RequestFailedException or its subtype from UpdateStatus, WaitForCompletion, or WaitForCompletionAsync.
  • Include any relevant error state information in the exception details.

We have a mix of some operations that only throw from the Value property - specifically the get_Value accessor - and some that throw but don't include the response or the HTTP status code, though the latter is a bit confusing because it should be 200 OK according to the REST API guidelines.

Examples of some differences:

• azure-sdk-for-net/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/CertificateOperation.cs

  Lines 72 to 95 in 2bffa0b

  	public override KeyVaultCertificateWithPolicy Value
  	{
  	#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations
  	get
  	{
  	if (Properties is null)
  	{
  	throw new InvalidOperationException("The operation was deleted so no value is available.");
  	}
  	if (Properties.Status == CancelledStatus)
  	{
  	throw new OperationCanceledException("The operation was canceled so no value is available.");
  	}
  	if (Properties.Error != null)
  	{
  	throw new InvalidOperationException($"The certificate operation failed: {Properties.Error.Message}");
  	}
  	return OperationHelpers.GetValue(ref _value);
  	}
  	#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations
  	}

  : this was discussed in depth with architects and approved; at the time, we didn't want UpdateStatus throwing.

• azure-sdk-for-net/sdk/core/Azure.Core/src/Shared/ProtocolOperation.cs

  Lines 57 to 71 in 2bffa0b

  	async ValueTask<OperationState<T>> IOperation<T>.UpdateStateAsync(bool async, CancellationToken cancellationToken)
  	{
  	var state = await _nextLinkOperation.UpdateStateAsync(async, cancellationToken).ConfigureAwait(false);
  	if (state.HasSucceeded)
  	{
  	return OperationState<T>.Success(state.RawResponse, _resultSelector(state.RawResponse));
  	}
  	if (state.HasCompleted)
  	{
  	return OperationState<T>.Failure(state.RawResponse, state.OperationFailedException);
  	}
  	return OperationState<T>.Pending(state.RawResponse);
  	}

  : doesn't throw if the status operation indicates failure. Maybe higher up the stack?

I came across what seemed another 1 or 2 patterns, but having trouble remembering which they were / find them again.