feat(stepfunctions): Prevent launching Workflow Studio with invalid JSON + save metric refactoring #6024

witness-me · 2024-11-15T01:14:31Z

Problem

Currently, the Workflow Studio (WFS) webview opens even when the JSON definition is invalid, which causes errors in the webview since WFS requires an initial, valid JSON definition.
The current metric for saving the file lacks a clear name and is not optimally defined for tracking save/sync events.
There is an issue with the undo functionality (CTRL+Z) inside WFS where the webview definition does not reflect changes in the local file. This occurs because VSCode's built-in undo command interferes with the WFS undo.
The production CDN link is missing in the code (since it was not set up before)

Solution

Detect invalid JSON files and prevent WFS from opening if JSON is invalid. In such cases, switch to the default editor and display a warning message.
Refactor and rename the save/sync metric for clarity and modify its parameters for improved tracking.
Suppress VSCode's built-in undo command when WFS webview is focused to prevent conflicts, allowing WFS to manage undo operations directly.
Production CDN link is added

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

github-actions · 2024-11-15T01:14:49Z

This pull request modifies code in src/ but no tests were added/updated. Confirm whether tests should be added or ensure the PR description explains why tests are not required.

github-actions · 2024-11-15T01:14:49Z

This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions.

witness-me · 2024-11-15T01:21:39Z

Demo recording

jpinkney-aws · 2024-11-20T13:32:59Z

packages/core/src/shared/telemetry/vscodeTelemetry.json

+        {
+            "name": "stepFunctionsSyncType",
+            "type": "string",
+            "allowedValues": ["MANUAL_SAVE_WFS", "MANUAL_SAVE_VSCODE", "AUTO_SYNC_WFS"],


is there any reason we can't use camel case here?

I used it this way because the previous version of the metric that I was using (saveType introduced by Threat Composer integration) had the same screaming snake casing.

I can update the metric to camel case to keep this consistent with most of the types

jpinkney-aws · 2024-11-20T13:34:22Z

packages/core/package.json

@@ -3653,6 +3653,12 @@
                "key": "ctrl+shift+v",
                "mac": "cmd+shift+v",
                "when": "editorTextFocus && editorLangId == asl || editorTextFocus && editorLangId == asl-yaml"
+            },
+            {
+                "command": "noop",


Can you explain more about why this is needed?

Sure thing!

In VSCode, the Workflow Studio integration works as a custom editor, so it inherits all the standard VSCode keybindings, including Ctrl+Z for undo. At the same time, WFS has its own undo keybinding (Ctrl+Z) to handle undo actions specifically within the WFS context, like those in the console.

However, these actions conflict with each other in the IDE. When the user presses Ctrl+Z, both WFS and the IDE try to perform undo actions simultaneously. This leads to a bug in the established logic: undoing via hotkeys in WFS applies the action within WFS but doesn’t properly reflect the changes in the local file, unlike regular edits or when undo is triggered using the WFS interface button.

The solution is to suppress VSCode’s default undo behavior when the WFS integration is active and focused, to prevent this interference. I tried to avoid adding new custom state for tracking and instead relied on context keys, but unfortunately, context keys don’t seem to be available for custom editors.

Makes sense. Is there any way to intercept the save command on the custom editor using something like

webView.webview.onDidRecieveMessage(e => { // whatever save does here })

I belive you mean undo in your comment, rather than save.

The command is executed inside WFS and only affects WFS internal definition, the webview does not receive any message for it.
With the suppression logic I introduced, it should work like this:

User triggers undo in WFS (from their keyboard or the button in the UI)

WFS performs its internal undo and updates its internal definition to the previous one

Updated WFS internal definition is synced with the local file

At the same time, VSCode built-in undo is suppressed for Ctrl+Z to not interfere with the above behaviour

I did not find any better suppression options since the custom editor (including webview) has all the standard VSCode keybindings working (such as Ctrl+Z) unless we override them

Ah interesting, I think that's fine then

Let me check with my team to see if they have any ideas how to do this without a noop command

## Problem - telemetry fields are not sorted and hard to compare ## Solution - sort alphabetically ---  License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

adding functionality for selective transformation to allow for java 17 as source and display multiple diff patches for the user to accept/reject

## Problem running `npm run compile` always gives you a toolkit package.json change because the icons are out of sync ## Solution commit the updated toolkit icons ---  License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

## Problem The `sync.ts` are huge consisting of many helper functions that being reused for deploy and build. ## Solution - split shared methods into separate files for easier testing and maintenance - split sync.test.ts file into smaller file - consolidate `paramsSourcePrompter` for both sync and deploy to avoid duplicate code - update PrompterTester helper class to clean up VS Code UI element for early exit from consumer callback - rename paramsSource prompter title --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Roger Zhang <[email protected]>

aws#6055) ## Problem Our backend is expecting the language name (`java` or `sql`) to be lowercase, not uppercase. Also, the failure reason returned by one of our APIs was being overwritten and not shown to users. ## Solution User lowercase, and prevent overwriting.

- Simplify when to focus the notifications panel for emergency notifications. - Try to limit race conditions when dismissing notifications.

…6058) ## Problem If customers open a workspace without a workspace folder they get: ``` 2024-11-19 10:49:16.189 [error] Error: No workspace folders found at k (/Users/myuser/.vscode/extensions/amazonwebservices.amazon-q-vscode-99.0.0-g3afd54d/dist/src/extensionNode.js:5464:5562) at L.buildIndex (/Users/myuser/.vscode/extensions/amazonwebservices.amazon-q-vscode-99.0.0-g3afd54d/dist/src/extensionNode.js:5464:9890) at Immediate.<anonymous> (/Users/myuser/.vscode/extensions/amazonwebservices.amazon-q-vscode-99.0.0-g3afd54d/dist/src/extensionNode.js:5464:11854) ``` because workspace indexing is throwing an error ## Solution Log that no workspace folders were found so indexing is skipped

## Problem Update dependencies to avoid any vulnerabilities ## Solution

fix(notifications): extension activation hangs if fetch fails

…ing session (aws#6063) ## Problem Currently after closing a session in a way that leaves the TextDocument open (`Stop tailing` CodeLens), the codeLenses remain visible. This means a user could click `Stop tailing` again, and receive an error for not being able to find the running LiveTail session for the given document. Additionally, there is no feedback when a session is closed. Clicking `Stop tailing` doesn't signal to the user that the session was actually stopped. ## Solution * Provide a `refresh` method in the LiveTail CodeLens provider. This fires an event to force recomputing the CodeLenses on a document. * Modify LiveTail Lens provider to return no Lenses if the session is not running (in the registry) * Display an information window when a Session is stopped * Changes wording/placement on some log statements for consistency.

- Removes feature flag - Slightly adjusts toolkit_invokeAction telemetry to not use button string, instead we use action enums. ---  License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

… returns empty (aws#6053) ## Problem there are 2 scenarios where we consider "Empty" 1. empty list of suggestion : `[]` 2. non empty list of suggestion with empty content: `['', '']` Currently, only (2) will have plugins send out `userTriggerDecision` event whereas (1) doesn't. ## Solution ---  License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

…ation (aws#6072)

…alid JSON metric

…r timeout (aws#6256) ## Problem related issue: aws#6079, aws#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 ![image](https://github.com/user-attachments/assets/f59c11cf-0e34-40b8-8162-34b4d057673f) ``` 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 aws#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 (aws#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.

…nd test timeouts aws#6258 ## Problem ref aws#6252 Test is using real system clock, it's likely to timeout due to heavy computation in an undeterministic way. ## Solution Use fake clock.

## 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.

…s#6267 ## 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.

fix aws#6266

## 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]>

## Problem fix aws#6213 ## Solution - The important part of this test is that the message comes after being inactive. If the message comes slightly early/late, that is okay. If it doesn't come at all, then we have problems and should fail. - Pass test if message comes within a minute of expected.

Problem: Since 6dcbfc7, the `lint-commits` job runs whenever the PR *description* is edited, which then re-runs all the other GHA jobs. This is because it listens to the `edited` event, which triggers whenever the PR is edited in any way (very noisy). Solution: There is no way to avoid rerunning all the CI jobs if they are dependent on `lint-commits`. Instead, revert 6dcbfc7 and document that re-opening the PR will force CI to re-run, if needed, after fixing a PR title.

## Problem Code block extends beyond margins followed by the rest of the document. ## Solution Remove css styles

## Problem - mynah ui 4.21.3 contains a fix needed for tests: https://github.com/aws/mynah-ui/releases/tag/v4.21.3 ## Solution - update to it. No public facing changes are needed

witness-me requested a review from a team as a code owner November 15, 2024 01:14

jpinkney-aws reviewed Nov 20, 2024

View reviewed changes

Will-ShaoHua and others added 24 commits November 20, 2024 09:00

Merge master into feature/cwltail

e0506f3

deps(amazonq): Update mynah-ui to 4.18.1 (aws#6060)

817ebac

Merge master into feature/cwltail

933c8de

feat(amazon q): display multiple diff patches (aws#5812)

ac72bca

adding functionality for selective transformation to allow for java 17 as source and display multiple diff patches for the user to accept/reject

Merge master into feature/cwltail

2936445

Merge master into feature/cwltail

b5e719f

Merge master into feature/cwltail

5be294e

fix(notifications): dismiss race conditions

540ce54

- Simplify when to focus the notifications panel for emergency notifications. - Try to limit race conditions when dismissing notifications.

deps(core): Update dependencies (aws#6061)

db9efb7

## Problem Update dependencies to avoid any vulnerabilities ## Solution

Merge master into feature/cwltail

adac49d

Merge pull request aws#6052 from hayemaxi/notifications6

f355557

fix(notifications): extension activation hangs if fetch fails

Merge master into feature/cwltail

833cc7f

Merge master into feature/cwltail

443990e

Merge master into feature/cwltail

4b52b87

fix(amazon q): updating string for final patch of selective transform…

cde861c

…ation (aws#6072)

Merge master into feature/cwltail

d42a6bf

witness-me force-pushed the wfs-invalid-json branch from 645124c to 3c166d7 Compare November 21, 2024 01:34

witness-me force-pushed the wfs-invalid-json branch 2 times, most recently from cc41bc0 to 6a7abfc Compare December 16, 2024 22:21

feat(stepfunctions): handle launching with invalid JSON file, add inv…

9c40f4c

…alid JSON metric

witness-me force-pushed the wfs-invalid-json branch from 6a7abfc to 9c40f4c Compare December 16, 2024 22:27

Will-ShaoHua and others added 21 commits December 16, 2024 14:39

Release 1.41.0

17dbcd3

Release 3.40.0

f556380

Update version to snapshot version: 3.41.0-SNAPSHOT

39327fb

Update version to snapshot version: 1.42.0-SNAPSHOT

08ce242

Merge release into master

4a0187d

test(inline-suggestion): crossFileContextUtil.test using real clock a…

7084800

…nd test timeouts aws#6258 ## Problem ref aws#6252 Test is using real system clock, it's likely to timeout due to heavy computation in an undeterministic way. ## Solution Use fake clock.

fix(amazonq): allow postgresql as target DB aws#6259

b085dd9

## Problem `RDS_POSTGRESQL` should be spelled as `POSTGRESQL` ## Solution Fix spelling

refactor(settings): non-async isPromptEnabled() aws#6255

04a5b68

## 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.

test(inline-suggestion): supplementalContextUtil.test failure aws#6269

a3fdfca

fix aws#6266

fix(amazonq): increase GetTransformation maxRetries aws#6220

ee2ac96

## 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.

docs: mention EC2

cfa53f1

fix(amazonq): code block extends beyond margins aws#6253

08c7a32

## Problem Code block extends beyond margins followed by the rest of the document. ## Solution Remove css styles

deps(amazonq): update mynah ui to 4.21.3 (aws#6276)

df7a3b6

## Problem - mynah ui 4.21.3 contains a fix needed for tests: https://github.com/aws/mynah-ui/releases/tag/v4.21.3 ## Solution - update to it. No public facing changes are needed

deps(stepfunctions): merge branch 'master' into the feature branch

1701d49

witness-me requested review from a team as code owners December 19, 2024 21:10

justinmk3 merged commit 901bfbe into aws:feature/stepfunctions-workflow Dec 19, 2024
16 of 17 checks passed

witness-me deleted the wfs-invalid-json branch December 20, 2024 02:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(stepfunctions): Prevent launching Workflow Studio with invalid JSON + save metric refactoring #6024

feat(stepfunctions): Prevent launching Workflow Studio with invalid JSON + save metric refactoring #6024

witness-me commented Nov 15, 2024 •

edited

Loading

github-actions bot commented Nov 15, 2024

github-actions bot commented Nov 15, 2024

witness-me commented Nov 15, 2024

jpinkney-aws Nov 20, 2024

witness-me Nov 21, 2024

jpinkney-aws Nov 20, 2024

witness-me Nov 21, 2024 •

edited

Loading

jpinkney-aws Nov 21, 2024

witness-me Nov 22, 2024

jpinkney-aws Nov 22, 2024

jpinkney-aws Nov 22, 2024

feat(stepfunctions): Prevent launching Workflow Studio with invalid JSON + save metric refactoring #6024

feat(stepfunctions): Prevent launching Workflow Studio with invalid JSON + save metric refactoring #6024

Conversation

witness-me commented Nov 15, 2024 • edited Loading

Problem

Solution

github-actions bot commented Nov 15, 2024

github-actions bot commented Nov 15, 2024

witness-me commented Nov 15, 2024

jpinkney-aws Nov 20, 2024

Choose a reason for hiding this comment

witness-me Nov 21, 2024

Choose a reason for hiding this comment

jpinkney-aws Nov 20, 2024

Choose a reason for hiding this comment

witness-me Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

jpinkney-aws Nov 21, 2024

Choose a reason for hiding this comment

witness-me Nov 22, 2024

Choose a reason for hiding this comment

jpinkney-aws Nov 22, 2024

Choose a reason for hiding this comment

jpinkney-aws Nov 22, 2024

Choose a reason for hiding this comment

witness-me commented Nov 15, 2024 •

edited

Loading

witness-me Nov 21, 2024 •

edited

Loading