-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
The current implementation of @azure/core-lro does not provide out of the box support for LROs that return status monitors using the operation-location header. Instead, they only work when the azure-asyncoperation header is used, which was dictated by previous versions of the Azure REST API guidelines. However, the latest guidelines state that services should use the operation-location header for returning the status monitor URL; and only use azure-asyncoperation header in services that had previously included it, alongside an operation-location header with the same value.
Additionally, the guidelines indicate that client code should look for both of the headers to ensure compatibility: https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#long-running-operations-with-status-monitor
This can cause issues when using the default LRO implementation against APIs that only return the operation-location header, since the locationPolling strategy is used instead of the azureAsyncPolling strategy. This incorrectly uses the URL provided in the location header as the polling URL, instead of it being used as the URL for the final result after the operation is complete.
I believe that the algorithm that infers the LRO mode should be updated to take into account that some LROs return status monitors in the operation-location header only; currently, it only checks for status monitors in the azure-asyncoperation header:
azure-sdk-for-js/sdk/core/core-lro/src/lroEngine/requestUtils.ts
Lines 33 to 61 in 2923269
| export function inferLroMode( | |
| requestPath: string, | |
| requestMethod: string, | |
| rawResponse: RawResponse | |
| ): LroConfig { | |
| if (getAzureAsyncOperation(rawResponse) !== undefined) { | |
| return { | |
| mode: "AzureAsync", | |
| resourceLocation: | |
| requestMethod === "PUT" | |
| ? requestPath | |
| : requestMethod === "POST" || requestMethod === "PATCH" | |
| ? getLocation(rawResponse) | |
| : undefined, | |
| }; | |
| } else if ( | |
| getLocation(rawResponse) !== undefined || | |
| getOperationLocation(rawResponse) !== undefined | |
| ) { | |
| return { | |
| mode: "Location", | |
| }; | |
| } else if (["PUT", "PATCH"].includes(requestMethod)) { | |
| return { | |
| mode: "Body", | |
| }; | |
| } | |
| return {}; | |
| } |
For now, this issue can be addressed by manually setting the azure-asyncoperation header to the same value as the operation-location in the request before it is read by the Poller. This can be achieved by implementing a pipeline policy for the clients that have this problem. An example can be found in the @azure/communication-phone-numbers package:
azure-sdk-for-js/sdk/communication/communication-phone-numbers/src/utils/customPipelinePolicies.ts
Lines 47 to 59 in 0b5233e
| export const phoneNumbersLroPolicy: PipelinePolicy = { | |
| name: "phoneNumbersLroPolicy", | |
| async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> { | |
| const response = await next(request); | |
| const operationLocation = response.headers?.get("operation-location"); | |
| if (operationLocation) { | |
| response.headers.set("azure-asyncoperation", operationLocation); | |
| } | |
| return response; | |
| }, | |
| }; |