Merge master into feature/q-dev-execution #6178
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merge master into feature/cwltail
Problem As LiveTail session is running, we want to display to the user metadata about their session. Solution While a livetail session is the active editor, in the bottom status bar, we will display the duration the session has been running for, how many events per/s, and if the log data is being sampled or not.
## Problem Users may want to clear their screen during a tailing session. The liveTail document is read only. Users may want to stop their session without closing the document. ## Solution Provide a codeLens to clear the screen (make document empty) Provide a codeLens to stop the session without closing the document. Moves Interval Timer for updating the StatusBar to the LiveTailSession object so `stopLiveTailSession()` can interrupt it, and be guaranteed to be cleaned up. The TextDocument's URI and Session URI seem to not be equal. Looking up in the LiveTailSessionRegistry with a document URI causes a session to not be found. Converting with `toString` allows these URIs to match and the registry to work as intended. Improves Exception handling on-stream. Previously, stopping the session in an expected fashion (codeLens, Closing editors) would cause an exception to bubble up and appear to the User. New change recognizes when the Abort Controller triggers, signifying an error has not occured, and logs the event as opposed to surfacing an error. Exposing only `isAborted` so that a caller has to use `stopLiveTailSession` to trigger the abortController and guarantee that other clean up actions have taken place.
## Problem
Exceptions can occur pre-stream (the synchronous portion of a
StartLiveTail call that establishes the streaming connection).
Currently, we are calling StartLiveTail in a try-catch, catching
errrors, and throwing them as a ToolkitException. These are not chaining
the root exception. This means when an error occurs, its root cause is
being swallowed - causing user's to not know *why* their LiveTall
command is failing.
## Solution
Given that we are just rethrowing `err`. There's probably no point to
this catch. Removing it, and letting the root exception throw.
Forced pre-stream exception to throw with an IAM permission violation
and this change applied. More clear as to what the actual problem is:
Pop-up: `Failed to run command: aws.cwl.tailLogGroup: User:
arn:aws:sts::203607498903:assumed-role/NoLiveTail/keegani-Isengard is
not authorized to perform: logs:StartLiveTail on resource:
arn:aws:logs:us-east-1:203607498903:log-group:/aws/codebuild/BATSSandboxCodeBuildPr-bf0a23097fbc3948a2c5b26f1616f7d32b622cba
because no identity-based policy allows the logs:StartLiveTail action`
Full log:
```
2024-11-11 13:00:04.310 [error] aws.cwl.tailLogGroup: [AccessDeniedException: User: arn:aws:sts::203607498903:assumed-role/NoLiveTail/keegani-Isengard is not authorized to perform: logs:StartLiveTail on resource: arn:aws:logs:us-east-1:203607498903:log-group:/aws/codebuild/BATSSandboxCodeBuildPr-bf0a23097fbc3948a2c5b26f1616f7d32b622cba because no identity-based policy allows the logs:StartLiveTail action
at de_AccessDeniedExceptionRes (/Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/node_modules/@aws-sdk/client-cloudwatch-logs/dist-cjs/index.js:2249:21)
at de_CommandError (/Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/node_modules/@aws-sdk/client-cloudwatch-logs/dist-cjs/index.js:2203:19)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async /Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/node_modules/@aws-sdk/client-cloudwatch-logs/node_modules/@smithy/middleware-serde/dist-cjs/index.js:35:20
at async /Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/node_modules/@smithy/core/dist-cjs/index.js:168:18
at async /Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/node_modules/@aws-sdk/client-cloudwatch-logs/node_modules/@smithy/middleware-retry/dist-cjs/index.js:320:38
at async /Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/node_modules/@aws-sdk/client-cloudwatch-logs/node_modules/@aws-sdk/middleware-logger/dist-cjs/index.js:34:22
at async LiveTailSession.startLiveTailSession (/Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/packages/core/dist/src/awsService/cloudWatchLogs/registry/liveTailSession.js:70:31)
at async tailLogGroup (/Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/packages/core/dist/src/awsService/cloudWatchLogs/commands/tailLogGroup.js:58:20)
at async /Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/packages/core/dist/src/awsService/cloudWatchLogs/activation.js:91:9
at async runCommand (/Users/keegani/workplace/aws-toolkit-vscode-release/aws-toolkit-vscode/packages/core/dist/src/shared/vscode/commands2.js:445:16)
at async Y0.h (file:///Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:114:32825)] {
'$fault': 'client',
'$metadata': [Object],
__type: 'AccessDeniedException'
}
```
---
<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->
License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.
Co-authored-by: Keegan Irby <[email protected]>
…#6113 ## Problem contributors may use the low-level node `child_process` library, which lacks the instrumentation and robustness of the toolkit `ChildProcess` utility. Example: #5925 (comment) ## Solution - add the node `child_process` library as a "banned import" with a message pointing to our `ChildProcess` utility. - Migrate simple cases to use our `ChildProcess`, mark more complex ones as exceptions. ## Future Work - To migrate the existing cases marked as exceptions, we will need to add functionality to our `ChildProcess` api, such as enforcing a `maxBufferSize` on the output. - Additionally, some of the current uses of `child_process` are tied to implementation details in the `child_process` library that make it hard to change without a larger-scale refactor. These cases were marked as exceptions.
## Problem Performance test was reliant on dead code removed in #6218 ## Solution - update thresholds to accommodate the change. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Problem: Error users were seeing was: ``` Error: SSO cache data unexpectedly missing props: ["refreshToken"] ``` This was due to a change from a previous PR that assumed the refreshToken was always present in SSO cache. ## Solution: It looks like the refreshToken does not exist for all cases, so some research needs to be done. But for now this reverts the validation that the refreshToken exists. Though it keeps the validation that the accessToken exists since that is always guaranteed. **A temporary workaround should be to sign out/in and things should start working. If not please downgrade 3.38.0 until we release this fix next week.** Fixes #6230 --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: nkomonen-amazon <[email protected]>
) ## Problem If a user runs a transformation, rejects changes, then immediately runs another transformation on the same project (rare, but it happens), we attempt to show the user the old diff (which has already been deleted at this point). ## Solution Reset `patchFiles` before showing the user the patch file. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license. Co-authored-by: David Hasani <[email protected]>
## Problem - the current implementation only opens the first tab as a welcome tab for the next 3 times you open vscode ## Solution - the new implementation shows the welcome tab 3 total times, either when you open a new chat tab or a new vscode window or both --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Problem - For the last 8 months or so we've skipped e2e test results, now they're becoming a lot more crucial and used by other teams ## Solution - remove the skipping --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
…to always use repomap (#6205) ## Problem Remove client side A/B experiment (control, treatment1, treatment2), move it to service side and always assume treatment1 ## Solution --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
… telemetry (#6242) ## Problem Current `supplementalContextStrategy ` only reflect what group the user is in, however by doing this it won't help much on our data analysis. What we need is the context strategy being used so that we know how much % of users have repomap or opentabs and how % of users have empty supplemental context etc. ## Solution --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Problem `jobId` missing in some metrics. ## Solution Add it.
## Problem Apply fix removes other issues in the same file. This is happening because apply fix command will always replace the entire file contents which triggers a document change event that intersects with all issues in the file. ## Solution Look through the diff hunks and apply them in a range. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Problem Techdebt test is failing CI. This is unlikely to be addressed atm. ## Solution - Delay until 2025.
…points #6240 ## Problem SAM CLI guided deploy support template parameter override for both sync and deploy command. However, the AppBuilder wizard UI only support this feature for SAM deploy trigger from SAM template context menu or AppBuilder project node menu button. ## Solution - Implement nested wizard for template parameter for both sync and deploy action for all entry points. - Refactor DeployWizard class for consistency with SyncWizard class - Add unit test for validating correct backward flow and state restoration for both wizard.
…r timeout (#6256) ## Problem related issue: #6079, #6252 caller ``` function main () { // init vscode cancellation token const cancellationToken setTimeout(100, () => { cancellationToken.cancel() }) highlevelWrapperFetchSupplementalContext(editor, cancellationToken) } ``` ``` export function highlevelWrapperFetchSupplementalContext(editor, cancellationToken) { const supplementalContext = waitUntil(100, () => { // here always timeout and throw TimeoutException const opentabs = await fetchOpenTabsContext(...) const projectContext = await fetchProjectContext() const result = [] if (projectContext not empty) { // push project context } if (opentabs not empty) {} // push openttabs }) return result } async function fetchOpenTabsContext(editor, cancellationToken) { .... // VSC api call } async function fetchProjectContext() { .... // LSP call } ``` After investigation, it looks like mix use of `vscode.CancellationToken` and `waitUntil()` will likely cause cancellation token to be cancelled prematurely (might be because another layer of waitUntil will run the fetchOpenTabsContext asynchronously thus causing it to timeout prematurely) therefore `fetchOpebtabsContext(..)` will return null in this case and hence causing test cases failing. Therefore, the issue here is actually not the test case itself and they're failing due to race condition ## Solution remove usage of cancellation token and only use waitUntil for timeout purpose ## Functional testing retrieved sup context as expected ### Case 1: repomap is available (there are local imports) ``` 2024-12-16 13:10:15.616 [debug] CodeWhispererSupplementalContext: isUtg: false, isProcessTimeout: false, contentsLength: 14436, latency: 16.67179101705551 strategy: codemap Chunk 0: Path: q-inline Length: 10209 Score: 0 Chunk 1: Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/service/serviceContainer.ts Length: 1486 Score: 22.60257328587725 Chunk 2: Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts Length: 1649 Score: 19.106700952807103 Chunk 3: Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts Length: 1092 Score: 10.334690655691002 ``` ### Case 2: No repomap, should fallback to opentabs only  ``` 2024-12-16 13:11:29.738 [debug] CodeWhispererSupplementalContext: isUtg: false, isProcessTimeout: false, contentsLength: 5046, latency: 16.311500012874603 strategy: opentabs Chunk 0: Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts Length: 1564 Score: 0 Chunk 1: Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts Length: 1649 Score: 0 Chunk 2: Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/tracker/lineTracker.ts Length: 1833 Score: 0 ``` --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Problem #6225 - Didn't think 400% could be exceeded, but this one is especially volatile. Sometimes it barely spikes, and sometimes it goes crazy. ## Solution - Increase to 500% --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
… logs (#6263) ## Problem - JSDOM logs non-critical CSS parsing errors during E2E tests. These warnings occur when mynah-ui loads SCSS files in [main.ts](https://github.com/aws/mynah-ui/blob/8058f86ed370f5268ba6e0993b2436dca0e09b30/src/main.ts#L37). These errors appear the test logs but they do not affect test functionality or results. ## Solution - Suppress the errors --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
## Problem `RDS_POSTGRESQL` should be spelled as `POSTGRESQL` ## Solution Fix spelling
## Problem There is no real reason this needs to be async. It is also now inconsistent with `isExperimentEnabled`. ## Solution Make non-async and add logged error if the promise rejects.
## Problem Currently, the TailLogGroup tests are passing, but emitting noisy errors in the CI logs: ``` <...> No LiveTail session found for URI: test-region:test-log-group:all: Error: No LiveTail session found for URI: test-region:test-log-group:all at D:\a\aws-toolkit-vscode\aws-toolkit-vscode\packages\core\src\awsService\cloudWatchLogs\commands\tailLogGroup.ts:94:19 at AsyncLocalStorage.run (node:async_hooks:346:14) <...> ``` The cause for this, is that the event listener for closing the tailing session when Editor tabs close in TailLogGroup is not being disposed of. Disposal happens [here](https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts#L80). However, currently the "mock response stream" in the test blocks indefinitely on an [un-resolvable promise](https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/test/awsService/cloudWatchLogs/commands/tailLogGroup.test.ts#L67). So the test ends, and this finally block never triggers. ## Solution Instead of using an un-resolving promise to keep the mock responseStream open, use an AbortController. When the tests no longer need the functionality of the disposables, the test can fire the AbortController. This causes the stream to exit exceptionally, which triggers the `finally` block in TailLogGroup and runs disposal. This stops the event listeners from running after the test is completed, and has eliminated the noisy logs. I have tested this by running `npm run test` from the CLI.
## Problem Customers that experience transient network issues are unable to view their transformation results when the `GetTransformation` API fails after the default of 3 retries. ## Solution Increase retries to 8.
## Problem S3 upload sometimes fails due to transient issues. ## Solution Add up to 3 retries. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: David Hasani <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Automatic merge failed
Command line hint
To perform the merge from the command line, you could do something like the following (where "origin" is the name of the remote in your local git repo):