fix(amazonq): add cancel support to loading developer profiles #940
Conversation
| @@ -80,7 +80,7 @@ export const QConfigurationServerToken = | |||
| case Q_CONFIGURATION_SECTION: | |||
| ;[customizations, developerProfiles] = await Promise.all([ | |||
| serverConfigurationProvider.listAvailableCustomizations(), | |||
| serverConfigurationProvider.listAvailableProfiles(), | |||
| serverConfigurationProvider.listAvailableProfiles(token), | |||
There was a problem hiding this comment.
I think we should cancel whole onGetConfigurationFromServer request and return RequestCanceled LSP Error from here to signal that client cancellation was handled by server correctly.
* The client has canceled a request and a server has detected
* the cancel.
*/
export const RequestCancelled: [integer](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#integer) = -32800;
There was a problem hiding this comment.
Agreed, we could for all switch cases here pass the token to a method e.g. handleTokenCancellation, which will throw a RequestCancelled error in case the request was cancelled.
There was a problem hiding this comment.
Sure -- I'm going to refrain from adding it to the Q_CUSTOMIZATIONS_CONFIGURATION_SECTION case statement, because customizations code currently does not support tokens.
| @@ -73,6 +84,10 @@ async function fetchProfilesFromRegion( | |||
| do { | |||
| logging.debug(`Fetching profiles from region: ${region} (iteration: ${numberOfPages})`) | |||
|
|
|||
| if (token.isCancellationRequested) { | |||
| return allRegionalProfiles | |||
There was a problem hiding this comment.
nit: I think we could just return an empty list here, request might be cancelled after some profiles are already aggregated due to pagination and higher up we we will return an empty list after cancellation anyways
There was a problem hiding this comment.
IMO this is a concern for the caller, whether the caller is within flare or from a client. In both places, the caller can decide what to do given that they have access to the token cancellation state.
…oken is cancelled
Problem
There can be some latency while loading developer profiles, caused by either a poor internet connection, a sluggish backend, or both. Clients need a way to cancel the request for loading profiles.
Solution
Wire in support for the Cancellation Token.
This change is being done in tandem with VS Toolkit change https://github.com/aws/aws-toolkit-visual-studio-staging/pull/2443
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.