RequestContext.CancellationToken is never used

[heaths]

For APIs that expose a RequestContext instead of a CancellationToken, users can only pass a CancellationToken by setting RequestContext.CancellationToken; however, VS shows 0 references to RequestContext.CancellationToken and walking through the pipeline code, everything seems to read HttpMessage.CancellationToken which is only set by HttpPipeline.Send and SendAsync - not something most users are really meant to call themselves.

Looking at generated client code like

azure-sdk-for-net/sdk/cognitivelanguage/Azure.AI.Language.Conversations/src/Generated/ConversationAnalysisClient.cs

Lines 141 to 157 in 49cf1a2

	internal virtual async Task<Operation<BinaryData>> StartAnalyzeConversationAsync(WaitUntil waitUntil, RequestContent content, RequestContext context = null)
	{
	Argument.AssertNotNull(content, nameof(content));
	using var scope = ClientDiagnostics.CreateScope("ConversationAnalysisClient.StartAnalyzeConversation");
	scope.Start();
	try
	{
	using HttpMessage message = CreateStartAnalyzeConversationRequest(content, context);
	return await ProtocolOperationHelpers.ProcessMessageAsync(_pipeline, message, ClientDiagnostics, "ConversationAnalysisClient.StartAnalyzeConversation", OperationFinalStateVia.Location, context, waitUntil).ConfigureAwait(false);
	}
	catch (Exception e)
	{
	scope.Failed(e);
	throw;
	}
	}

and its request factory like

azure-sdk-for-net/sdk/cognitivelanguage/Azure.AI.Language.Conversations/src/Generated/ConversationAnalysisClient.cs

Lines 248 to 263 in 49cf1a2

	internal HttpMessage CreateStartAnalyzeConversationRequest(RequestContent content, RequestContext context)
	{
	var message = _pipeline.CreateMessage(context, ResponseClassifier200202);
	var request = message.Request;
	request.Method = RequestMethod.Post;
	var uri = new RawRequestUriBuilder();
	uri.Reset(_endpoint);
	uri.AppendRaw("/language", false);
	uri.AppendPath("/analyze-conversations/jobs", false);
	uri.AppendQuery("api-version", _apiVersion, true);
	request.Uri = uri;
	request.Headers.Add("Accept", "application/json");
	request.Headers.Add("Content-Type", "application/json");
	request.Content = content;
	return message;
	}

I also see nowhere that RequestContext.CancellationToken is used; thus, it seems impossible for customers to cancel any methods taking a RequestContext.

[heaths]

I found this while changing GetSecretIfExists using a CancellationToken to a GetSecret overload taking a RequestContext for PR #33767, which is more robust. To work around this, in my custom pipeline (mainly just wrappers) I'm passing context?.CancellationToken ?? default to HttpPipeline.Send and SendAsync but it seems this shouldn't be necessary. Perhaps HttpPipeline.CreateMessage should set HttpMessage.CancellationToken from the optionally passed-in RequestContext.CancellationToken.

[heaths]

@annelo-msft pointed out that the RequestContext.CancellationToken is getting used via an extension method. Indeed, I'm seeing this get called in CLU which calls ProtocolOperationHelpers.ProcessMessage:

azure-sdk-for-net/sdk/cognitivelanguage/Azure.AI.Language.Conversations/src/Generated/ConversationAnalysisClient.cs

Line 176 in 49cf1a2

return ProtocolOperationHelpers.ProcessMessage(_pipeline, message, ClientDiagnostics, "ConversationAnalysisClient.StartAnalyzeConversation", OperationFinalStateVia.Location, context, waitUntil);

However, if you call HttpPipeline.Send or SendAsync directly, this does not appear to be the case. Still seems it would be more robust to have HttpPipeline.CreateMessage set the CancellationToken as mentioned in my comment above since all code seems to be calling that:

azure-sdk-for-net/sdk/cognitivelanguage/Azure.AI.Language.Conversations/src/Generated/ConversationAnalysisClient.cs

Line 250 in 49cf1a2

var message = _pipeline.CreateMessage(context, ResponseClassifier200202);

[annelo-msft]

@heaths, given this finding, is there still an issue in CLU? Or only in KeyVault where the pipeline is being called manually? What libraries are actually affected?

[heaths]

No, it seems there's no issue with CLU. Only when trying to add a convenience method taking a RequestContext, which @KrzysztofCwalina, @tg-msft, and @m-nash approved for this; though there's another potential blocker to #33767 outside this problem.

[annelo-msft]

Could the extension method be called in the convenience method, or is that not possible?

[heaths]

Sure it could, but now there's an implicit dependency people just have to know to use or behavior may vary (and it's not like many libraries seem to test cancellation). What's wrong with making sure it's always set in a call that everyone has to use anyway like HttpPipeline.CreateMessage?

[annelo-msft]

Nothing is wrong with it, and I agree it's a nicer approach. I ask primarily to evaluate whether you're blocked or not, as a means of prioritizing this relative to other tasks.