Merge branch 'master' into feature/separate-sessions

Author: hayemaxi

packages/core/src/codewhisperer/activation.ts

@@ -50,7 +50,7 @@ import { ReferenceInlineProvider } from './service/referenceInlineProvider'
 import { disposeSecurityDiagnostic, securityScanRender } from './service/diagnosticsProvider'
 import { SecurityPanelViewProvider, openEditorAtRange } from './views/securityPanelViewProvider'
 import { RecommendationHandler } from './service/recommendationHandler'
-import { Commands, registerCommandsWithVSCode } from '../shared/vscode/commands2'
+import { Commands, registerCommandErrorHandler, registerDeclaredCommands } from '../shared/vscode/commands2'
 import { InlineCompletionService, refreshStatusBar } from './service/inlineCompletionService'
 import { isInlineCompletionEnabled } from './util/commonUtil'
 import { CodeWhispererCodeCoverageTracker } from './tracker/codewhispererCodeCoverageTracker'
@@ -68,7 +68,7 @@ import { Container } from './service/serviceContainer'
 import { debounceStartSecurityScan } from './commands/startSecurityScan'
 import { securityScanLanguageContext } from './util/securityScanLanguageContext'
 import { registerWebviewErrorHandler } from '../webviews/server'
-import { logAndShowWebviewError } from '../shared/utilities/logAndShowUtils'
+import { logAndShowError, logAndShowWebviewError } from '../shared/utilities/logAndShowUtils'
 import { openSettings } from '../shared/settings'
 import { getCodeCatalystDevEnvId } from '../shared/vscode/env'
 
@@ -93,8 +93,9 @@ export async function activate(context: ExtContext): Promise<void> {
         await enableDefaultConfigCloud9()
     }
 
-    registerCommandsWithVSCode(
-        context.extensionContext,
+    // TODO: is this indirection useful?
+    registerDeclaredCommands(
+        context.extensionContext.subscriptions,
         CodeWhispererCommandDeclarations.instance,
         new CodeWhispererCommandBackend(context.extensionContext)
     )
@@ -105,9 +106,13 @@ export async function activate(context: ExtContext): Promise<void> {
     const securityPanelViewProvider = new SecurityPanelViewProvider(context.extensionContext)
     activateSecurityScan()
 
-    /**
-     * Register the webview error handler for Amazon Q
-     */
+    // TODO: this is already done in packages/core/src/extensionCommon.ts, why doesn't amazonq use that?
+    registerCommandErrorHandler((info, error) => {
+        const defaultMessage = localize('AWS.generic.message.error', 'Failed to run command: {0}', info.id)
+        void logAndShowError(localize, error, info.id, defaultMessage)
+    })
+
+    // TODO: this is already done in packages/core/src/extensionCommon.ts, why doesn't amazonq use that?
     registerWebviewErrorHandler((error: unknown, webviewId: string, command: string) => {
         logAndShowWebviewError(localize, error, webviewId, command)
     })
@@ -304,7 +309,8 @@ export async function activate(context: ExtContext): Promise<void> {
 
     if (auth.isConnectionExpired()) {
         auth.showReauthenticatePrompt().catch(e => {
-            getLogger().error('showReauthenticatePrompt failed: %s', (e as Error).message)
+            const defaulMsg = localize('AWS.generic.message.error', 'Failed to reauth:')
+            void logAndShowError(localize, e, 'showReauthenticatePrompt', defaulMsg)
         })
     }
     if (auth.isValidEnterpriseSsoInUse()) {

packages/core/src/extensionCommon.ts

@@ -35,7 +35,7 @@ import { CredentialsStore } from './auth/credentials/store'
 import { initializeAwsCredentialsStatusBarItem } from './auth/ui/statusBarItem'
 import { RegionProvider, getEndpointsFromFetcher } from './shared/regions/regionProvider'
 import { getMachineId } from './shared/vscode/env'
-import { registerErrorHandler as registerCommandErrorHandler } from './shared/vscode/commands2'
+import { registerCommandErrorHandler } from './shared/vscode/commands2'
 import { registerWebviewErrorHandler } from './webviews/server'
 import { showQuickStartWebview } from './shared/extensionStartup'
 import { ExtContext } from './shared/extensions'

packages/core/src/shared/errors.ts

@@ -13,6 +13,7 @@ import { isNonNullable } from './utilities/tsUtils'
 import type * as fs from 'fs'
 import type * as os from 'os'
 import { CodeWhispererStreamingServiceException } from '@amzn/codewhisperer-streaming'
+import { isAutomation } from './vscode/env'
 
 export const errorCode = {
     invalidConnection: 'InvalidConnection',
@@ -82,7 +83,8 @@ export interface ErrorInformation {
     /**
      * A link to documentation relevant to this error.
      *
-     * TODO: implement this
+     * TODO: use this throughout the codebase.
+     * TODO: prefer `Error.error_uri` if present (from OIDC/OAuth service)?
      */
     readonly documentationUri?: vscode.Uri
 }
@@ -234,9 +236,13 @@ export function getErrorMsg(err: Error | undefined): string | undefined {
         return undefined
     }
 
-    // error_description is a non-standard SDK field added by (at least) OIDC service.
-    // If present, it has better information, so prefer it to `message`.
-    // https://github.com/aws/aws-toolkit-jetbrains/commit/cc9ed87fa9391dd39ac05cbf99b4437112fa3d10
+    // Non-standard SDK fields added by the OIDC service, to conform to the OAuth spec
+    // (https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1) :
+    // - error: code per the OAuth spec
+    // - error_description: improved error message provided by OIDC service. Prefer this to
+    //   `message` if present.
+    //   https://github.com/aws/aws-toolkit-jetbrains/commit/cc9ed87fa9391dd39ac05cbf99b4437112fa3d10
+    // - error_uri: not provided by OIDC currently?
     //
     // Example:
     //
@@ -355,16 +361,20 @@ const _preferredErrors: RegExp[] = [
     /^ResourceNotFoundException$/,
     /^ServiceQuotaExceededException$/,
     /^AccessDeniedException$/,
+    /^InvalidPermissions$/,
+    /^EPIPE$/,
+    /^EPERM$/,
 ]
 
 /**
  * Searches the `cause` chain (if any) for the most useful/relevant {@link AWSError} to surface to
  * the user, preferring "deeper" errors (lower-level, closer to the root cause) when all else is equal.
  *
  * These conditions determine precedence (in order):
- * - required: AWSError type
- * - `error_description` field
- * - `code` matches one of `preferredErrors`
+ * - is AWSError
+ * - has `error_description` field
+ * - has `code` matching `preferredErrors`
+ * - is filesystem error
  * - cause chain depth (the deepest error wins)
  *
  * @param error Error whose `cause` chain will be searched.
@@ -375,14 +385,26 @@ const _preferredErrors: RegExp[] = [
 export function findBestErrorInChain(error: Error, preferredErrors = _preferredErrors): Error | undefined {
     // TODO: Base Error has 'cause' in ES2022. So does our own `ToolkitError`.
     // eslint-disable-next-line @typescript-eslint/naming-convention
-    let bestErr: Error & { cause?: Error; error_description?: string } = error
+    let bestErr: Error & { code?: string; cause?: Error; error_description?: string } = error
     let err: typeof bestErr | undefined
 
-    for (let i = 0; (err || i === 0) && i < 100; i++) {
+    for (let i = 0; i < 100; i++) {
         err = i === 0 ? bestErr.cause : err?.cause
+        if (!err) {
+            break
+        }
 
-        if (isAwsError(err)) {
-            if (!isAwsError(bestErr)) {
+        // const bestErrCode = bestErr.code?.trim() ?? ''
+        // const preferBest = ...
+        const errCode = err.code?.trim() ?? ''
+        const prefer =
+            (errCode !== '' && preferredErrors.some(re => re.test(errCode))) ||
+            // In priority order:
+            isFilesystemError(err) ||
+            isNoPermissionsError(err)
+
+        if (isAwsError(err) || (prefer && !isAwsError(bestErr))) {
+            if (isAwsError(err) && !isAwsError(bestErr)) {
                 bestErr = err // Prefer AWSError.
                 continue
             }
@@ -393,11 +415,7 @@ export function findBestErrorInChain(error: Error, preferredErrors = _preferredE
                 continue
             }
 
-            // const bestErrCode = bestErr.code?.trim() ?? ''
-            // const bestErrMatches = bestErrCode !== '' && preferredErrors.some(re => re.test(bestErrCode))
-            const errCode = err.code?.trim() ?? ''
-            const errMatches = errCode !== '' && preferredErrors.some(re => re.test(errCode))
-            if (!bestErr.error_description && errMatches) {
+            if (!bestErr.error_description && prefer) {
                 bestErr = err
             }
         }
@@ -469,6 +487,41 @@ export function getRequestId(err: unknown): string | undefined {
     }
 }
 
+export function isFilesystemError(err: unknown): boolean {
+    if (
+        err instanceof vscode.FileSystemError ||
+        (hasCode(err) &&
+            (err.code === 'EEXIST' ||
+                err.code === 'EISDIR' ||
+                err.code === 'ENOTDIR' ||
+                err.code === 'EMFILE' ||
+                err.code === 'ENOENT' ||
+                err.code === 'ENOTEMPTY'))
+    ) {
+        return true
+    }
+
+    return false
+}
+
+// export function isIsDirError(err: unknown): boolean {
+//     if (err instanceof vscode.FileSystemError) {
+//         return err.code === vscode.FileSystemError.FileIsADirectory().code
+//     } else if (hasCode(err)) {
+//         return err.code === 'EISDIR'
+//     }
+//     return false
+// }
+//
+// export function isFileExistsError(err: unknown): boolean {
+//     if (err instanceof vscode.FileSystemError) {
+//         return err.code === vscode.FileSystemError.FileExists().code
+//     } else if (hasCode(err)) {
+//         return err.code === 'EEXIST'
+//     }
+//     return false
+// }
+
 export function isFileNotFoundError(err: unknown): boolean {
     if (err instanceof vscode.FileSystemError) {
         return err.code === vscode.FileSystemError.FileNotFound().code
@@ -487,16 +540,33 @@ export function isNoPermissionsError(err: unknown): boolean {
             (err.code === 'Unknown' && err.message.includes('EACCES: permission denied'))
         )
     } else if (hasCode(err)) {
+        // " Some operating systems report EPERM in situations unrelated to permissions."
+        // || err.code === 'EPERM'
         return err.code === 'EACCES'
     }
 
     return false
 }
 
-const modeToString = (mode: number) =>
-    Array.from('rwxrwxrwx')
+function modeToString(mode: number) {
+    return Array.from('rwxrwxrwx')
         .map((c, i, a) => ((mode >> (a.length - (i + 1))) & 1 ? c : '-'))
         .join('')
+}
+
+function vscodeModeToString(mode: vscode.FileStat['permissions']) {
+    // XXX: vscode.FileStat.permissions only indicates "readonly" or nothing (aka "writable").
+    if (mode === undefined) {
+        return 'rwx------'
+    } else if (mode === vscode.FilePermission.Readonly) {
+        return 'r-x------'
+    }
+
+    // XXX: future-proof in case vscode.FileStat.permissions gains more granularity.
+    if (isAutomation()) {
+        throw new Error('vscode.FileStat.permissions gained new fields, update this logic')
+    }
+}
 
 function getEffectivePerms(uid: number, gid: number, stats: fs.Stats) {
     const mode = stats.mode
@@ -529,38 +599,71 @@ export type PermissionsTriplet = `${'r' | '-' | '*'}${'w' | '-' | '*'}${'x' | '-
 export class PermissionsError extends ToolkitError {
     public readonly actual: string // This is a resolved triplet, _not_ the full bits
 
+    static fromNodeFileStats(stats: fs.Stats, userInfo: os.UserInfo<string>, expected: string, source: unknown) {
+        const mode = `${stats.isDirectory() ? 'd' : '-'}${modeToString(stats.mode)}`
+        const owner = stats.uid === userInfo.uid ? (stats.uid === -1 ? '' : userInfo.username) : String(stats.uid)
+        const group = String(stats.gid)
+        const { effective, isAmbiguous } = getEffectivePerms(userInfo.uid, userInfo.gid, stats)
+        const actual = modeToString(effective).slice(-3)
+        const isOwner = stats.uid === -1 ? 'unknown' : userInfo.uid === stats.uid
+
+        return { mode, owner, group, actual, isAmbiguous, isOwner }
+    }
+
+    static fromVscodeFileStats(
+        stats: vscode.FileStat,
+        userInfo: os.UserInfo<string>,
+        expected: string,
+        source: unknown
+    ) {
+        const isDir = !!(stats.type & vscode.FileType.Directory)
+        const mode = `${isDir ? 'd' : '-'}${vscodeModeToString(stats.permissions)}`
+        const owner = '' // vscode.FileStat does not currently provide file owner.
+        const group = '' // vscode.FileStat does not currently provide file group.
+        const isAmbiguous = true // vscode.workspace.fs.stat() is currently always ambiguous.
+        const actual = mode
+        const isOwner = 'unknown' // vscode.FileStat does not currently provide file owner.
+
+        return { mode, owner, group, actual, isAmbiguous, isOwner }
+    }
+
+    /**
+     * Creates a PermissionsError from a file stat() result.
+     *
+     * Note: pass `fs.Stats` when possible (in a nodejs context), because it gives much better info.
+     */
     public constructor(
         public readonly uri: vscode.Uri,
-        public readonly stats: fs.Stats,
+        public readonly stats: fs.Stats | vscode.FileStat,
         public readonly userInfo: os.UserInfo<string>,
         public readonly expected: PermissionsTriplet,
         source?: unknown
     ) {
-        const mode = `${stats.isDirectory() ? 'd' : '-'}${modeToString(stats.mode)}`
-        const owner = stats.uid === userInfo.uid ? userInfo.username : stats.uid
-        const { effective, isAmbiguous } = getEffectivePerms(userInfo.uid, userInfo.gid, stats)
-        const actual = modeToString(effective).slice(-3)
+        const o = (stats as any).type
+            ? PermissionsError.fromVscodeFileStats(stats as vscode.FileStat, userInfo, expected, source)
+            : PermissionsError.fromNodeFileStats(stats as fs.Stats, userInfo, expected, source)
+
         const resolvedExpected = Array.from(expected)
-            .map((c, i) => (c === '*' ? actual[i] : c))
+            .map((c, i) => (c === '*' ? o.actual[i] : c))
             .join('')
-        const actualText = !isAmbiguous ? actual : `${mode.slice(-6, -3)} & ${mode.slice(-3)} (ambiguous)`
+        const actualText = !o.isAmbiguous ? o.actual : `${o.mode.slice(-6, -3)} & ${o.mode.slice(-3)} (ambiguous)`
 
         // Guard against surfacing confusing error messages. If the actual perms equal the resolved
         // perms then odds are it wasn't really a permissions error. Some operating systems report EPERM
         // in situations that aren't related to permissions at all.
-        if (actual === resolvedExpected && !isAmbiguous && source !== undefined) {
+        if (o.actual === resolvedExpected && !o.isAmbiguous && source !== undefined) {
             throw source
         }
 
         super(`${uri.fsPath} has incorrect permissions. Expected ${resolvedExpected}, found ${actualText}.`, {
             code: 'InvalidPermissions',
             details: {
-                isOwner: stats.uid === -1 ? 'unknown' : userInfo.uid === stats.uid,
-                mode: `${mode}${stats.uid === -1 ? '' : ` ${owner}`}${stats.gid === -1 ? '' : ` ${stats.gid}`}`,
+                isOwner: o.isOwner,
+                mode: `${o.mode}${o.owner === '' ? '' : ` ${o.owner}`}${o.group === '' ? '' : ` ${o.group}`}`,
             },
         })
 
-        this.actual = actual
+        this.actual = o.actual
     }
 }
 

packages/core/src/shared/extensionUtilities.ts

@@ -41,6 +41,11 @@ export function productName() {
     return isAmazonQ() ? 'Amazon Q' : `${getIdeProperties().company} Toolkit`
 }
 
+/** Gets the "AWS" or "Amazon Q" prefix (in package.json: `commands.category`). */
+export function commandsPrefix(): string {
+    return isAmazonQ() ? 'Amazon Q' : getIdeProperties().company
+}
+
 export const mostRecentVersionKey: string = 'globalsMostRecentVersion'
 
 export enum IDE {

packages/core/src/shared/localizedText.ts

@@ -4,7 +4,7 @@
  */
 
 import * as nls from 'vscode-nls'
-import { getIdeProperties } from './extensionUtilities'
+import { commandsPrefix, getIdeProperties } from './extensionUtilities'
 const localize = nls.loadMessageBundle()
 
 export const yes = localize('AWS.generic.response.yes', 'Yes')
@@ -35,8 +35,8 @@ export function connectionExpired(name: string) {
 export const checklogs = () =>
     localize(
         'AWS.error.check.logs',
-        'Check the logs by running the "View {0} Toolkit Logs" command from the {1}.',
-        getIdeProperties().company,
+        'Check the logs by running the "{0}: View Logs" command from the {1}.',
+        commandsPrefix(),
         getIdeProperties().commandPalette
     )
 

packages/core/src/shared/vscode/commands2.ts

@@ -113,14 +113,12 @@ export interface CommandDeclarations<T> {
  * @param declarations Has the mapping of command names to the backend logic
  * @param backend The backend logic of the commands
  */
-export function registerCommandsWithVSCode<T>(
-    extContext: vscode.ExtensionContext,
+export function registerDeclaredCommands<T>(
+    disposables: { dispose(): any }[],
     declarations: CommandDeclarations<T>,
     backend: T
 ): void {
-    extContext.subscriptions.push(
-        ...Object.values<DeclaredCommand>(declarations.declared).map(c => c.register(backend))
-    )
+    disposables.push(...Object.values<DeclaredCommand>(declarations.declared).map(c => c.register(backend)))
 }
 
 /**
@@ -674,7 +672,7 @@ async function runCommand<T extends Callback>(fn: T, info: CommandInfo<T>): Prom
 // the extension entry-point. `Commands` form the backbone of everything else in the Toolkit.
 // This file should contain as little application-specific logic as possible.
 let errorHandler: (info: Omit<CommandInfo<any>, 'args'>, error: unknown) => void
-export function registerErrorHandler(handler: typeof errorHandler): void {
+export function registerCommandErrorHandler(handler: typeof errorHandler): void {
     if (errorHandler !== undefined) {
         throw new TypeError('Error handler has already been registered')
     }