fix(amazonq): retry S3 upload (#6246)

## 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 <davhasan@amazon.com>

packages/amazonq/.changes/next-release/Bug Fix-f7bb3f35-1b43-4604-b3bd-c2e7f1b65083.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Amazon Q Code Transformation: retry project upload up to 3 times"
+}

packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts

@@ -252,7 +252,7 @@ export class Messenger {
         this.dispatcher.sendAsyncEventProgress(
             new AsyncEventProgressMessage(tabID, {
                 inProgress: true,
-                message: MessengerUtils.createLanguageUpgradeConfirmationPrompt(detectedJavaVersions),
+                message: CodeWhispererConstants.projectPromptChatMessage,
             })
         )
 

packages/core/src/amazonqGumby/chat/controller/messenger/messengerUtils.ts

@@ -69,30 +69,8 @@ export default class MessengerUtils {
         }
     }
 
-    static createLanguageUpgradeConfirmationPrompt = (detectedJavaVersions: Array<JDKVersion | undefined>): string => {
-        let javaVersionString = 'Java project'
-        const uniqueJavaOptions = new Set(detectedJavaVersions)
-
-        if (detectedJavaVersions.length > 1) {
-            // this  means there is a Java version whose version we weren't able to determine
-            if (uniqueJavaOptions.has(undefined)) {
-                javaVersionString = 'Java projects'
-            } else {
-                javaVersionString = `Java ${Array.from(uniqueJavaOptions).join(' & ')} projects`
-            }
-        } else if (detectedJavaVersions.length === 1) {
-            if (!uniqueJavaOptions.has(undefined)) {
-                javaVersionString = `Java ${detectedJavaVersions[0]!.toString()} project`
-            }
-        }
-
-        return CodeWhispererConstants.projectPromptChatMessage.replace('JAVA_VERSION_HERE', javaVersionString)
-    }
-
     static createAvailableDependencyVersionString = (versions: DependencyVersions): string => {
-        let message = `I found ${versions.length} other dependency versions that are more recent than the dependency in your code that's causing an error: ${versions.currentVersion}.
-
-`
+        let message = `I found ${versions.length} other dependency versions that are more recent than the dependency in your code that's causing an error: ${versions.currentVersion}.`
 
         if (versions.majorVersions !== undefined && versions.majorVersions.length > 0) {
             message = message.concat(

packages/core/src/codewhisperer/models/constants.ts

@@ -747,7 +747,7 @@ export const cleanInstallErrorNotification = `Amazon Q could not run the Maven c
 export const enterJavaHomeChatMessage = 'Enter the path to JDK '
 
 export const projectPromptChatMessage =
-    'I can upgrade your JAVA_VERSION_HERE. To start the transformation, I need some information from you. Choose the project you want to upgrade and the target code version to upgrade to. Then, choose Confirm.'
+    'I can upgrade your Java project. To start the transformation, I need some information from you. Choose the project you want to upgrade and the target code version to upgrade to. Then, choose Confirm.'
 
 export const windowsJavaHomeHelpChatMessage =
     'To find the JDK path, run the following commands in a new terminal: `cd "C:/Program Files/Java"` and then `dir`. If you see your JDK version, run `cd <version>` and then `cd` to show the path.'

packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts

@@ -103,18 +103,45 @@ export async function uploadArtifactToS3(
     try {
         const uploadFileByteSize = (await nodefs.promises.stat(fileName)).size
         getLogger().info(
-            `Uploading project artifact at %s with checksum %s using uploadId: %s and size %s kB`,
+            `CodeTransformation: Uploading project artifact at %s with checksum %s using uploadId: %s and size %s kB`,
             fileName,
             sha256,
             resp.uploadId,
             Math.round(uploadFileByteSize / 1000)
         )
 
-        const response = await request.fetch('PUT', resp.uploadUrl, {
-            body: buffer,
-            headers: getHeadersObj(sha256, resp.kmsKeyArn),
-        }).response
-        getLogger().info(`CodeTransformation: Status from S3 Upload = ${response.status}`)
+        let response = undefined
+        /* The existing S3 client has built-in retries but it requires the bucket name, so until
+         * CreateUploadUrl can be modified to return the S3 bucket name, manually implement retries.
+         * Alternatively, when waitUntil supports a fixed number of retries and retriableCodes, use that.
+         */
+        const retriableCodes = [408, 429, 500, 502, 503, 504]
+        for (let i = 0; i < 4; i++) {
+            try {
+                response = await request.fetch('PUT', resp.uploadUrl, {
+                    body: buffer,
+                    headers: getHeadersObj(sha256, resp.kmsKeyArn),
+                }).response
+                getLogger().info(`CodeTransformation: upload to S3 status on attempt ${i + 1}/4 = ${response.status}`)
+                if (response.status === 200) {
+                    break
+                }
+                throw new Error('Upload failed')
+            } catch (e: any) {
+                if (response && !retriableCodes.includes(response.status)) {
+                    throw new Error(`Upload failed with status code = ${response.status}; did not automatically retry`)
+                }
+                if (i !== 3) {
+                    await sleep(1000 * Math.pow(2, i))
+                }
+            }
+        }
+        if (!response || response.status !== 200) {
+            const uploadFailedError = `Upload failed after up to 4 attempts with status code = ${response?.status ?? 'unavailable'}`
+            getLogger().error(`CodeTransformation: ${uploadFailedError}`)
+            throw new Error(uploadFailedError)
+        }
+        getLogger().info('CodeTransformation: Upload to S3 succeeded')
     } catch (e: any) {
         let errorMessage = `The upload failed due to: ${(e as Error).message}. For more information, see the [Amazon Q documentation](${CodeWhispererConstants.codeTransformTroubleshootUploadError})`
         if (errorMessage.includes('Request has expired')) {

packages/core/src/codewhisperer/service/transformByQ/transformProjectValidationHandler.ts

@@ -2,12 +2,8 @@
  * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
  * SPDX-License-Identifier: Apache-2.0
  */
-import { BuildSystem, JDKVersion, TransformationCandidateProject } from '../../models/model'
-import { getLogger } from '../../../shared/logger'
-import * as CodeWhispererConstants from '../../models/constants'
+import { BuildSystem, TransformationCandidateProject } from '../../models/model'
 import * as vscode from 'vscode'
-// Consider using ChildProcess once we finalize all spawnSync calls
-import { spawnSync } from 'child_process' // eslint-disable-line no-restricted-imports
 import {
     NoJavaProjectsFoundError,
     NoMavenJavaProjectsFoundError,
@@ -70,72 +66,9 @@ async function getMavenJavaProjects(javaProjects: TransformationCandidateProject
     return mavenJavaProjects
 }
 
-async function getProjectsValidToTransform(mavenJavaProjects: TransformationCandidateProject[]) {
-    const projectsValidToTransform: TransformationCandidateProject[] = []
-    for (const project of mavenJavaProjects) {
-        let detectedJavaVersion = undefined
-        const projectPath = project.path
-        const compiledJavaFiles = await vscode.workspace.findFiles(
-            new vscode.RelativePattern(projectPath!, '**/*.class'),
-            '**/node_modules/**',
-            1
-        )
-        if (compiledJavaFiles.length > 0) {
-            const classFilePath = `${compiledJavaFiles[0].fsPath}`
-            const baseCommand = 'javap'
-            const args = ['-v', classFilePath]
-            const spawnResult = spawnSync(baseCommand, args, { shell: false, encoding: 'utf-8' })
-            if (spawnResult.status !== 0) {
-                let errorLog = ''
-                errorLog += spawnResult.error ? JSON.stringify(spawnResult.error) : ''
-                errorLog += `${spawnResult.stderr}\n${spawnResult.stdout}`
-                getLogger().error(`CodeTransformation: Error in running javap command = ${errorLog}`)
-                let errorReason = ''
-                if (spawnResult.stdout) {
-                    errorReason = 'JavapExecutionError'
-                } else {
-                    errorReason = 'JavapSpawnError'
-                }
-                if (spawnResult.error) {
-                    const errorCode = (spawnResult.error as any).code ?? 'UNKNOWN'
-                    errorReason += `-${errorCode}`
-                }
-                getLogger().error(
-                    `CodeTransformation: Error in running javap command = ${errorReason}, log = ${errorLog}`
-                )
-            } else {
-                const majorVersionIndex = spawnResult.stdout.indexOf('major version: ')
-                const javaVersion = spawnResult.stdout.slice(majorVersionIndex + 15, majorVersionIndex + 17).trim()
-                if (javaVersion === CodeWhispererConstants.JDK8VersionNumber) {
-                    detectedJavaVersion = JDKVersion.JDK8
-                } else if (javaVersion === CodeWhispererConstants.JDK11VersionNumber) {
-                    detectedJavaVersion = JDKVersion.JDK11
-                } else {
-                    detectedJavaVersion = JDKVersion.UNSUPPORTED
-                }
-            }
-        }
-
-        // detectedJavaVersion will be undefined if there are no .class files or if javap errors out, otherwise it will be JDK8, JDK11, or UNSUPPORTED
-        project.JDKVersion = detectedJavaVersion
-        projectsValidToTransform.push(project)
-    }
-    return projectsValidToTransform
-}
-
-/*
- * This function filters all open projects by first searching for a .java file and then searching for a pom.xml file in all projects.
- * It also tries to detect the Java version of each project by running "javap" on a .class file of each project.
- * As long as the project contains a .java file and a pom.xml file, the project is still considered valid for transformation,
- * and we allow the user to specify the Java version.
- */
+// This function filters all open projects by first searching for a .java file and then searching for a pom.xml file in all projects.
 export async function validateOpenProjects(projects: TransformationCandidateProject[]) {
     const javaProjects = await getJavaProjects(projects)
-
     const mavenJavaProjects = await getMavenJavaProjects(javaProjects)
-
-    // These projects we know must contain a pom.xml and a .java file
-    const projectsValidToTransform = await getProjectsValidToTransform(mavenJavaProjects)
-
-    return projectsValidToTransform
+    return mavenJavaProjects
 }

packages/core/src/test/codewhisperer/commands/transformByQ.test.ts

@@ -42,8 +42,12 @@ import {
     parseBuildFile,
     validateSQLMetadataFile,
 } from '../../../codewhisperer/service/transformByQ/transformFileHandler'
+import { uploadArtifactToS3 } from '../../../codewhisperer/indexNode'
+import request from '../../../shared/request'
+import * as nodefs from 'fs' // eslint-disable-line no-restricted-imports
 
 describe('transformByQ', function () {
+    let fetchStub: sinon.SinonStub
     let tempDir: string
     const validSctFile = `<?xml version="1.0" encoding="UTF-8"?>
     <tree>
@@ -91,6 +95,10 @@ describe('transformByQ', function () {
     beforeEach(async function () {
         tempDir = (await TestFolder.create()).path
         transformByQState.setToNotStarted()
+        fetchStub = sinon.stub(request, 'fetch')
+        // use Partial to avoid having to mock all 25 fields in the response; only care about 'size'
+        const mockStats: Partial<nodefs.Stats> = { size: 1000 }
+        sinon.stub(nodefs.promises, 'stat').resolves(mockStats as nodefs.Stats)
     })
 
     afterEach(async function () {
@@ -464,4 +472,90 @@ describe('transformByQ', function () {
         const isValidMetadata = await validateSQLMetadataFile(sctFileWithInvalidTarget, { tabID: 'abc123' })
         assert.strictEqual(isValidMetadata, false)
     })
+
+    it('should successfully upload on first attempt', async () => {
+        const successResponse = {
+            ok: true,
+            status: 200,
+            text: () => Promise.resolve('Success'),
+        }
+        fetchStub.returns({ response: Promise.resolve(successResponse) })
+        await uploadArtifactToS3(
+            'test.zip',
+            { uploadId: '123', uploadUrl: 'http://test.com', kmsKeyArn: 'arn' },
+            'sha256',
+            Buffer.from('test')
+        )
+        sinon.assert.calledOnce(fetchStub)
+    })
+
+    it('should retry upload on retriable error and succeed', async () => {
+        const failedResponse = {
+            ok: false,
+            status: 503,
+            text: () => Promise.resolve('Service Unavailable'),
+        }
+        const successResponse = {
+            ok: true,
+            status: 200,
+            text: () => Promise.resolve('Success'),
+        }
+        fetchStub.onFirstCall().returns({ response: Promise.resolve(failedResponse) })
+        fetchStub.onSecondCall().returns({ response: Promise.resolve(successResponse) })
+        await uploadArtifactToS3(
+            'test.zip',
+            { uploadId: '123', uploadUrl: 'http://test.com', kmsKeyArn: 'arn' },
+            'sha256',
+            Buffer.from('test')
+        )
+        sinon.assert.calledTwice(fetchStub)
+    })
+
+    it('should throw error after 4 failed upload attempts', async () => {
+        const failedResponse = {
+            ok: false,
+            status: 500,
+            text: () => Promise.resolve('Internal Server Error'),
+        }
+        fetchStub.returns({ response: Promise.resolve(failedResponse) })
+        const expectedMessage =
+            'The upload failed due to: Upload failed after up to 4 attempts with status code = 500. For more information, see the [Amazon Q documentation](https://docs.aws.amazon.com/amazonq/latest/qdeveloper-ug/troubleshooting-code-transformation.html#project-upload-fail)'
+        await assert.rejects(
+            uploadArtifactToS3(
+                'test.zip',
+                { uploadId: '123', uploadUrl: 'http://test.com', kmsKeyArn: 'arn' },
+                'sha256',
+                Buffer.from('test')
+            ),
+            {
+                name: 'Error',
+                message: expectedMessage,
+            }
+        )
+        sinon.assert.callCount(fetchStub, 4)
+    })
+
+    it('should not retry upload on non-retriable error', async () => {
+        const failedResponse = {
+            ok: false,
+            status: 400,
+            text: () => Promise.resolve('Bad Request'),
+        }
+        fetchStub.onFirstCall().returns({ response: Promise.resolve(failedResponse) })
+        const expectedMessage =
+            'The upload failed due to: Upload failed with status code = 400; did not automatically retry. For more information, see the [Amazon Q documentation](https://docs.aws.amazon.com/amazonq/latest/qdeveloper-ug/troubleshooting-code-transformation.html#project-upload-fail)'
+        await assert.rejects(
+            uploadArtifactToS3(
+                'test.zip',
+                { uploadId: '123', uploadUrl: 'http://test.com', kmsKeyArn: 'arn' },
+                'sha256',
+                Buffer.from('test')
+            ),
+            {
+                name: 'Error',
+                message: expectedMessage,
+            }
+        )
+        sinon.assert.calledOnce(fetchStub)
+    })
 })