Skip to content

feat: add requestHandler ctor pass through type #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Nov 30, 2023

Currently, to override any options in the default request handler, the requestHandler class must be instantiated.

This adds a type representing default request handler ctor params,
intended to allow the following syntactic sugar:

import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { NodeHttpHandler } from "@smithy/node-http-handler";
import https from "https";    

var agent = new https.Agent({
  maxSockets: 25
});

var dynamodbClient = new DynamoDBClient({
  requestHandler: new NodeHttpHandler({
    httpsAgent: agent
 })
});

the PR's ultimate goal is to allow the following:

import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import https from "https";    

var agent = new https.Agent({
  maxSockets: 25
});

var dynamodbClient = new DynamoDBClient({
  requestHandler: {
    httpsAgent: agent
  }
});

This may appear to be a very mild simplification, but it avoids user errors like accidentally installing a locked version of @smithy/node-http-handler or needing to differentiate between the previous @aws-sdk/node-http-handler and its migrated name.

*
* @public
*/
export type RequestHandlerParams = NodeHttpHandlerOptions | FetchHttpHandlerOptions;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these types and interfaces live in @smithy/types, and the @smithy/*-http-handler packages import them so there is no duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's do that

@kuhe kuhe force-pushed the feat/requestHandler branch from 229451b to 0805d05 Compare November 30, 2023 21:50
@kuhe kuhe force-pushed the feat/requestHandler branch 2 times, most recently from f365812 to 08659ee Compare December 1, 2023 15:24
@kuhe kuhe force-pushed the feat/requestHandler branch from 08659ee to b253732 Compare December 1, 2023 15:44
@kuhe kuhe merged commit 340634a into smithy-lang:main Dec 1, 2023
@kuhe kuhe deleted the feat/requestHandler branch December 1, 2023 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants