Skip to content

Commit 2086649

Browse files
authored
refactor(http): inject same http handler into each sdk client. (#6690)
## Problem From #6664, we have persistent connections. However, each SDK client creates its own http handler. If we have N distinct service clients we maintain, then we could have up to N http handlers, with N distinct connection pools. This does not affect persistent connections since each service maintains its own endpoints, however, there is a small overhead in initiating each connection pool. Additionally, there is no guarantee for consistent behavior across handlers with regards to configuration options (Ex. requestTimeout). ## Solution - inject the same HTTP handler into each SDK client, unless explicitly given a different one. - use fetch-http-handler on web and `node-http-handler` on node. - We don't want to use `fetch-http-handler` in node because it still has experimental support and is not recommended. [docs](https://github.com/aws/aws-sdk-js-v3/blob/main/supplemental-docs/CLIENTS.md#request-handler-requesthandler) and [comment](aws/aws-sdk-js-v3#4619 (comment)) from SDK team. When trying to, there were issues with persistent connections. - install `http2` to resolve web deps issue. This is part of nodes standard library, but needed as explicit dependency for web. ### Trying `fetch-http-handler` in node. - confirmed `fetch-http-handler` with `keep-alive: true` option is sending `keepAlive` headers, but closing the connection after doing so in node, both in VSCode environment, and outside of it in a pure node environment. This implies it is not related to microsoft/vscode#173861. ## Verification The request times seemed unaffected by this change, but there was a noticeable impact on sdk client initialization speed. The results below are from creating 1000 SSM clients with and without the same HTTP Handler. <img width="304" alt="image" src="https://github.com/user-attachments/assets/9b28af43-795c-4dcb-9bb1-752c118a3247" /> Because we usually cache the SDK clients under each service, the important statistic is that this speeds up 0.131 ms per SDK client creation. If we always use the cache and only create a client once per service, then this also suggests a 0.131 ms per service speedup. We interact with at least 20 services, and 16 in the explorer alone, so this could result in 2-2.5 ms improvement in initialization time for all these SDK clients depending on how they are created. Could be interesting to revisit after the migration to see if this reduces start-up time. --- - 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.
1 parent eaadd25 commit 2086649

File tree

4 files changed

+162
-1
lines changed

4 files changed

+162
-1
lines changed

package-lock.json

Lines changed: 104 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/core/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,8 @@
521521
"@smithy/service-error-classification": "^3.0.0",
522522
"@smithy/shared-ini-file-loader": "^3.0.0",
523523
"@smithy/util-retry": "^3.0.0",
524+
"@smithy/fetch-http-handler": "^3.0.0",
525+
"@smithy/node-http-handler": "^3.0.0",
524526
"@vscode/debugprotocol": "^1.57.0",
525527
"@zip.js/zip.js": "^2.7.41",
526528
"adm-zip": "^0.5.10",
@@ -560,7 +562,8 @@
560562
"winston": "^3.11.0",
561563
"winston-transport": "^4.6.0",
562564
"xml2js": "^0.6.1",
563-
"yaml-cfn": "^0.3.2"
565+
"yaml-cfn": "^0.3.2",
566+
"http2": "^3.3.6"
564567
},
565568
"overrides": {
566569
"webfont": {

packages/core/src/shared/awsClientBuilderV3.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
RetryStrategy,
2626
UserAgent,
2727
} from '@aws-sdk/types'
28+
import { FetchHttpHandler } from '@smithy/fetch-http-handler'
2829
import { HttpResponse, HttpRequest } from '@aws-sdk/protocol-http'
2930
import { ConfiguredRetryStrategy } from '@smithy/util-retry'
3031
import { telemetry } from './telemetry/telemetry'
@@ -33,6 +34,8 @@ import { extensionVersion } from './vscode/env'
3334
import { getLogger } from './logger/logger'
3435
import { partialClone } from './utilities/collectionUtils'
3536
import { selectFrom } from './utilities/tsUtils'
37+
import { once } from './utilities/functionUtils'
38+
import { isWeb } from './extensionGlobals'
3639

3740
export type AwsClientConstructor<C> = new (o: AwsClientOptions) => C
3841

@@ -88,6 +91,20 @@ export class AWSClientBuilderV3 {
8891
return shim
8992
}
9093

94+
private buildHttpHandler() {
95+
const requestTimeout = 30000
96+
// HACK: avoid importing node-http-handler on web.
97+
return isWeb()
98+
? new FetchHttpHandler({ keepAlive: true, requestTimeout })
99+
: new (require('@smithy/node-http-handler').NodeHttpHandler)({
100+
httpAgent: { keepAlive: true },
101+
httpsAgent: { keepAlive: true },
102+
requestTimeout,
103+
})
104+
}
105+
106+
private getHttpHandler = once(this.buildHttpHandler.bind(this))
107+
91108
private keyAwsService<C extends AwsClient>(serviceOptions: AwsServiceOptions<C>): string {
92109
// Serializing certain objects in the args allows us to detect when nested objects change (ex. new retry strategy, endpoints)
93110
return [
@@ -129,6 +146,10 @@ export class AWSClientBuilderV3 {
129146
// Simple exponential backoff strategy as default.
130147
opt.retryStrategy = new ConfiguredRetryStrategy(5, (attempt: number) => 1000 * 2 ** attempt)
131148
}
149+
150+
if (!opt.requestHandler) {
151+
opt.requestHandler = this.getHttpHandler()
152+
}
132153
// TODO: add tests for refresh logic.
133154
opt.credentials = async () => {
134155
const creds = await shim.get()

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { Credentials, MetadataBearer, MiddlewareStack } from '@aws-sdk/types'
3232
import { oneDay } from '../../shared/datetime'
3333
import { ConfiguredRetryStrategy } from '@smithy/util-retry'
3434
import { StandardRetryStrategy } from '@smithy/util-retry'
35+
import { NodeHttpHandler } from '@smithy/node-http-handler'
3536

3637
describe('AwsClientBuilderV3', function () {
3738
let builder: AWSClientBuilderV3
@@ -77,6 +78,38 @@ describe('AwsClientBuilderV3', function () {
7778
assert.strictEqual(service.config.userAgent[0][0], 'CUSTOM USER AGENT')
7879
})
7980

81+
it('injects http client into handler', function () {
82+
const requestHandler = new NodeHttpHandler({
83+
requestTimeout: 1234,
84+
})
85+
const service = builder.createAwsService({
86+
serviceClient: Client,
87+
clientOptions: {
88+
requestHandler: requestHandler,
89+
},
90+
})
91+
assert.strictEqual(service.config.requestHandler, requestHandler)
92+
})
93+
94+
it('defaults to reusing singular http handler', function () {
95+
const service = builder.createAwsService({
96+
serviceClient: Client,
97+
})
98+
const service2 = builder.createAwsService({
99+
serviceClient: Client,
100+
})
101+
102+
const firstHandler = service.config.requestHandler
103+
const secondHandler = service2.config.requestHandler
104+
105+
// If not injected, the http handler can be undefined before making request.
106+
if (firstHandler instanceof NodeHttpHandler && secondHandler instanceof NodeHttpHandler) {
107+
assert.ok(firstHandler === secondHandler)
108+
} else {
109+
assert.fail('Expected both request handlers to be NodeHttpHandler instances')
110+
}
111+
})
112+
80113
describe('caching mechanism', function () {
81114
it('avoids recreating client on duplicate calls', async function () {
82115
const firstClient = builder.getAwsService({ serviceClient: TestClient })

0 commit comments

Comments
 (0)