test: update tests to avoid mocks

Author: Hweinstock

packages/core/src/shared/awsClientBuilderV3.ts

@@ -7,6 +7,10 @@ import { CredentialsShim } from '../auth/deprecated/loginManager'
 import { AwsContext } from './awsContext'
 import {
     AwsCredentialIdentityProvider,
+    BuildHandlerArguments,
+    DeserializeHandlerArguments,
+    DeserializeHandlerOutput,
+    FinalizeHandlerArguments,
     Logger,
     RetryStrategyV2,
     TokenIdentity,
@@ -213,26 +217,34 @@ export function recordErrorTelemetry(err: Error, serviceName?: string) {
 }
 
 export const defaultDeserializeMiddleware: DeserializeMiddleware<any, any> =
-    (next: DeserializeHandler<any, any>, context: HandlerExecutionContext) => async (args: any) =>
+    (next: DeserializeHandler<any, any>, context: HandlerExecutionContext) =>
+    async (args: DeserializeHandlerArguments<any>) =>
         onDeserialize(next, context, args)
 
 export const finalizeLoggingMiddleware: FinalizeRequestMiddleware<any, any> =
-    (next: FinalizeHandler<any, any>) => async (args: any) =>
+    (next: FinalizeHandler<any, any>) => async (args: FinalizeHandlerArguments<any>) =>
         logOnFinalize(next, args)
 
 function getEndpointMiddleware(settings: DevSettings = DevSettings.instance): BuildMiddleware<any, any> {
-    return (next: BuildHandler<any, any>, context: HandlerExecutionContext) => async (args: any) =>
-        overwriteEndpoint(next, context, settings, args)
+    return (next: BuildHandler<any, any>, context: HandlerExecutionContext) =>
+        async (args: BuildHandlerArguments<any>) =>
+            overwriteEndpoint(next, context, settings, args)
 }
 
-const keepAliveMiddleware: BuildMiddleware<any, any> = (next: BuildHandler<any, any>) => async (args: any) =>
-    addKeepAliveHeader(next, args)
+const keepAliveMiddleware: BuildMiddleware<any, any> =
+    (next: BuildHandler<any, any>) => async (args: BuildHandlerArguments<any>) =>
+        addKeepAliveHeader(next, args)
 
-export async function onDeserialize(next: DeserializeHandler<any, any>, context: HandlerExecutionContext, args: any) {
-    if (!HttpResponse.isInstance(args.request)) {
+export async function onDeserialize(
+    next: DeserializeHandler<any, any>,
+    context: HandlerExecutionContext,
+    args: DeserializeHandlerArguments<any>
+): Promise<DeserializeHandlerOutput<any>> {
+    const request = args.request
+    if (!HttpRequest.isInstance(request)) {
         return next(args)
     }
-    const { hostname, path } = args.request
+    const { hostname, path } = request
     const serviceId = getServiceId(context as object)
     const logTail = `(${hostname} ${path})`
     try {
@@ -247,15 +259,15 @@ export async function onDeserialize(next: DeserializeHandler<any, any>, context:
             const err = { ...e, name: e.name, mesage: e.message }
             delete err['stack']
             recordErrorTelemetry(err, serviceId)
-            getLogger().error('API Response %s: %O', logTail, err)
+            logErrorWithHeaders(request, err, logTail)
         }
         throw e
     }
 }
 
-export async function logOnFinalize(next: FinalizeHandler<any, any>, args: any) {
+export async function logOnFinalize(next: FinalizeHandler<any, any>, args: FinalizeHandlerArguments<any>) {
     const request = args.request
-    if (HttpRequest.isInstance(args.request)) {
+    if (HttpRequest.isInstance(request)) {
         const { hostname, path } = request
         const input = partialClone(args.input, 3, ['clientSecret', 'accessToken', 'refreshToken'], '[omitted]')
         getLogger().debug(`API Request (%s %s): %O`, hostname, path, input)
@@ -267,7 +279,7 @@ export function overwriteEndpoint(
     next: BuildHandler<any, any>,
     context: HandlerExecutionContext,
     settings: DevSettings,
-    args: any
+    args: BuildHandlerArguments<any>
 ) {
     const request = args.request
     if (HttpRequest.isInstance(request)) {
@@ -289,10 +301,25 @@ export function overwriteEndpoint(
  * @param args
  * @returns
  */
-export function addKeepAliveHeader(next: BuildHandler<any, any>, args: any) {
+export function addKeepAliveHeader(next: BuildHandler<any, any>, args: BuildHandlerArguments<any>) {
     const request = args.request
     if (HttpRequest.isInstance(request)) {
         request.headers['Connection'] = 'keep-alive'
     }
     return next(args)
 }
+
+export function logErrorWithHeaders(response: HttpRequest, err: Omit<Error, 'stack'>, logTail?: string) {
+    const logHeaderNames = [
+        'x-amzn-requestid',
+        'x-amzn-trace-id',
+        'x-amzn-served-from',
+        'x-cache',
+        'x-amz-cf-id',
+        'x-amz-cf-pop',
+    ]
+    const filteredHeaders = Object.fromEntries(
+        Object.entries(response.headers).filter(([k, v]) => logHeaderNames.includes(k))
+    )
+    getLogger().error('API request failed %s\n headers: %O\n %O', logTail, filteredHeaders, err)
+}

packages/core/src/test/shared/awsClientBuilderV3.test.ts

@@ -19,12 +19,12 @@ import {
     logOnFinalize,
     overwriteEndpoint,
     recordErrorTelemetry,
+    logErrorWithHeaders,
 } from '../../shared/awsClientBuilderV3'
 import { Client } from '@aws-sdk/smithy-client'
 import { DevSettings, extensionVersion } from '../../shared'
 import { assertTelemetry } from '../testUtil'
 import { telemetry } from '../../shared/telemetry'
-import { HttpRequest, HttpResponse } from '@aws-sdk/protocol-http'
 import { assertLogsContain, assertLogsContainAllOf } from '../globalSetup.test'
 import { TestSettings } from '../utilities/testSettingsConfiguration'
 import { CredentialsShim } from '../../auth/deprecated/loginManager'
@@ -33,6 +33,7 @@ import { oneDay } from '../../shared/datetime'
 import { ConfiguredRetryStrategy } from '@smithy/util-retry'
 import { StandardRetryStrategy } from '@smithy/util-retry'
 import { NodeHttpHandler } from '@smithy/node-http-handler'
+import { HttpRequest, HttpResponse } from '@smithy/protocol-http'
 
 describe('AwsClientBuilderV3', function () {
     let builder: AWSClientBuilderV3
@@ -197,55 +198,51 @@ describe('AwsClientBuilderV3', function () {
     })
 
     describe('middlewareStack', function () {
-        let args: { request: { hostname: string; path: string }; input: any }
+        let args: { request: HttpRequest; input: any }
         let context: { clientName?: string; commandName?: string }
-        let response: { response: { statusCode: number }; output: { message: string } }
-        let httpRequestStub: sinon.SinonStub
-        let httpResponseStub: sinon.SinonStub
-
-        before(function () {
-            httpRequestStub = sinon.stub(HttpRequest, 'isInstance')
-            httpResponseStub = sinon.stub(HttpResponse, 'isInstance')
-            httpRequestStub.callsFake(() => true)
-            httpResponseStub.callsFake(() => true)
-        })
+        let output: { response: HttpResponse }
 
         beforeEach(function () {
             args = {
-                request: {
+                request: new HttpRequest({
                     hostname: 'testHost',
                     path: 'testPath',
-                },
-                input: {
-                    testKey: 'testValue',
-                },
+                    headers: {
+                        'x-amzn-RequestId': 'fakeId',
+                        'x-amzn-requestid': 'realId',
+                    },
+                }),
+                input: {},
             }
-            context = {
-                clientName: 'fooClient',
-            }
-            response = {
-                response: {
+            output = {
+                response: new HttpResponse({
                     statusCode: 200,
-                },
-                output: {
-                    message: 'test output',
-                },
+                    body: 'test body',
+                    headers: {
+                        'x-amzn-RequestId': 'fakeId',
+                        'x-amzn-requestid': 'realId',
+                    },
+                }),
+            }
+            context = {
+                clientName: 'foo',
+                commandName: 'bar',
             }
         })
         after(function () {
             sinon.restore()
         })
 
         it('logs messages on request', async function () {
-            await logOnFinalize((_: any) => _, args as any)
+            await logOnFinalize((_: any) => _, args)
             assertLogsContainAllOf(['testHost', 'testPath'], false, 'debug')
         })
 
         it('adds telemetry metadata and logs on error failure', async function () {
             const next = (_: any) => {
                 throw new Error('test error')
             }
-            await telemetry.vscode_executeCommand.run(async (span) => {
+            await telemetry.vscode_executeCommand.run(async (_span) => {
                 await assert.rejects(onDeserialize(next, context, args))
             })
             assertLogsContain('test error', false, 'error')
@@ -254,10 +251,10 @@ describe('AwsClientBuilderV3', function () {
 
         it('does not emit telemetry, but still logs on successes', async function () {
             const next = async (_: any) => {
-                return response
+                return output
             }
-            await telemetry.vscode_executeCommand.run(async (span) => {
-                assert.deepStrictEqual(await onDeserialize(next, context, args), response)
+            await telemetry.vscode_executeCommand.run(async (_span) => {
+                assert.deepStrictEqual(await onDeserialize(next, context, args), output)
             })
             assertLogsContainAllOf(['testHost', 'testPath'], false, 'debug')
             assert.throws(() => assertTelemetry('vscode_executeCommand', { requestServiceType: 'foo' }))
@@ -281,7 +278,23 @@ describe('AwsClientBuilderV3', function () {
             const newArgs: any = await overwriteEndpoint(next, context, new DevSettings(settings), args)
 
             assert.strictEqual(newArgs.request.hostname, 'testHost')
-            assert.strictEqual(newArgs.request.path, 'testPath')
+            assert.strictEqual(newArgs.request.path, '/testPath')
+        })
+
+        it('logs specific headers on request failure', async function () {
+            const response = new HttpRequest({
+                body: 'test body',
+                headers: {
+                    'x-amzn-RequestId': 'fakeId',
+                    'x-amzn-requestid': 'realId',
+                    'x-amz-id-2': 'otherId',
+                },
+            })
+
+            logErrorWithHeaders(response, new Error('test error'), 'host-path')
+            assertLogsContain('realId', false, 'error')
+            assert.throws(() => assertLogsContain('fakeId', false, 'error'))
+            assert.throws(() => assertLogsContain('otherId', false, 'error'))
         })
     })