aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.07k stars 574 forks source link

Provide one class/interface for all clients #5856

Closed workeitel closed 5 months ago

workeitel commented 7 months ago

Describe the feature

Passing aws clients around in TypeScript requires knowing their type. This is easy for one client like DynamoDB but becomes impossible for any client. The fact that there is no single type makes for example middleware helpers difficult.

Use Case

Writing a middleware typically requires knowing the TypeScript type:

const client = new DynamoDB({ region: "us-west-2" });

addMyMiddleware(client);

But typing the addMyMiddleware is difficult as the type of an AWS Client is difficult to describe:

function addMyMiddleware(
    client: Client<
      HttpHandlerOptions,
      any, // defined as ClientInput extends object which is tricky to type as well
      MetadataBearer,
      SmithyResolvedConfiguration<HttpHandlerOptions> &
        RegionResolvedConfig &
        RetryResolvedConfig
    >
  ) { ... }

This is in particular bad as the types change between aws-sdk versions. For example Client was originally part of @aws-sdk/smithy-client but changed between versions to @smithy/smithy-client. Providing a addMyMiddleware method which works with all aws-sdk-js-v3 versions makes typing impossible.

Proposed Solution

I want to have one class/interface which describes all aws-sdk clients such as:

function addMyMiddleware(client: AWSClient) { ... }

This new type is stable across aws-sdk versions so the middleware does not need to be modified on aws-sdk updates.

Other Information

Writing a middleware for a specific aws-sdk-js-v3 version is not a problem. This feature is in particular helpful for vending middleware to many consumers with different aws-sdk-js-v3 versions.

Acknowledgements

SDK version used

all of them

Environment details (OS name and version, etc.)

all of them

kuhe commented 7 months ago

What parts of the client does your middleware interact with?

Why not use a structural subset?

Client is still exported from @aws-sdk/types. It's an alias of the one from @smithy.

workeitel commented 7 months ago

We own many different middleware integrations for different purpose. For example extracting logs/metrics from all aws-sdk calls.

I did not know that Client is reexported from @aws-sdk/types. That makes some things easier. But it still requires for example:

import { SmithyResolvedConfiguration } from "@smithy/smithy-client";
import { RegionResolvedConfig } from "@aws-sdk/config-resolver";
import { RetryResolvedConfig } from "@aws-sdk/middleware-retry";

function addMyMiddleware(
    client:Client<
      any,
      MetadataBearer,
      SmithyResolvedConfiguration<HttpHandlerOptions> &
        RegionResolvedConfig &
        RetryResolvedConfig
    >

the imports changed over time for example from @aws-sdk/smithy-client to @smithy/smithy-client similar as StandardRetryStrategy or AdaptiveRetryStrategy changed from @aws-sdk/util-retry to @smithy/util-retry.

kuhe commented 7 months ago

We do not have a base type because no particular behavior is guaranteed for a client based on the Smithy code generators, despite there being things that for practical purposes exist on all clients.

Below is an outline of what I recommend, which is to declare any required types of your middleware application function on that function's signature.

import type { LambdaClient } from "@aws-sdk/client-lambda";
import type { S3Client } from "@aws-sdk/client-s3";
import type { AwsCredentialIdentityProvider, Client } from "@smithy/types"; // same as Client @aws-sdk/types.

/**
 * The most generic client is one that has no restrictions on input/output,
 * and no requirements for the resolved config.
 */
type BaseClient = Client<any, any, {}>;

const s3 = null as any as S3Client;
const lambda = null as any as LambdaClient;

/**
 * SDK clients are assignable to this most generic client.
 */
let generic: BaseClient = s3;
generic = lambda;

/**
 * However, I would guess that you're looking for a configuration subset that is guaranteed for
 * all clients.
 *
 * We have not established this type because while Smithy code generation does generate certain
 * config fields for (in practice) all clients like systemClockOffset or requestHandler, it does so explicitly
 * for each client rather than to meet any particular base type.
 *
 * Therefore, there are no particular fields that are in fact guaranteed for all potential
 * clients.
 */

/**
 * In order to create a middleware applicator, you should specify exactly what you require.
 */
type MinimalGenericClient = BaseClient & {
  config: {
    credentials: AwsCredentialIdentityProvider;
    systemClockOffset: number;
  };
};

async function addMiddleware(client: MinimalGenericClient) {
  console.log(client.config.systemClockOffset);
  await client.config.credentials();
  client.middlewareStack.add(null as any, null as any);
  client.middlewareStack.addRelativeTo(null as any, null as any);
}

addMiddleware(s3);
addMiddleware(lambda);

/**
 * For handling changed internals, discern with union.
 */
type ImplementationAlphaIsAvailable = {
  config: {
    propertyA: AwsCredentialIdentityProvider; // pretend this only exists on some SDK versions.
  };
};
type ImplementationBetaIsAvailable = {
  config: {
    propertyB: AwsCredentialIdentityProvider; // pretend this only exists on other SDK versions.
  };
};
type ClientWithDivergentImplementations = BaseClient & (ImplementationAlphaIsAvailable | ImplementationBetaIsAvailable);

/**
 * Middleware applicator handling potentially divergent SDK config implementations.
 */
async function addMiddlewareChecked(client: ClientWithDivergentImplementations) {
  if (hasImplementationAlpha(client)) {
    await client.config.propertyA();
    client.middlewareStack.add(null as any, null as any);
  } else {
    await client.config.propertyB();
    client.middlewareStack.add(null as any, null as any);
  }
}

function hasImplementationAlpha(client: any): client is ImplementationAlphaIsAvailable {
  return !!client?.config?.propertyA;
}
workeitel commented 6 months ago

Thanks for this suggestion. That's a good temporary solution but I don't think this should be hard-coded by aws-sdk consumers. It might not be possible to provide something for generic smithy generated clients but at least for aws-sdk clients the list of config values and middleware stack is known (example: https://github.com/aws/aws-sdk-js-v3/blame/main/clients/client-appconfig/src/AppConfigClient.ts#L605-L628 ).

Why is it not possible to ship in @aws-sdk/types an interface to at least model the stable part of the contract. For example:

 export interface ServiceClient {
   config: {
     maxAttempts: () => Promise<number>;
     region: () => Promise<string>;
     retryStrategy: () => Promise<RetryStrategy | RetryStrategyV2>;
   };
   middlewareStack: {
     add: MiddlewareStack<any, MetadataBearer>["add"];
   };
 }

That does not solve the full problem because for example moving ServiceException from @aws-sdk/smithy-client to @smithy/smithy-client is still a breaking change which requires consumers to update their code on sdk update and makes a middleware which works across aws-sdk versions impossible. But at least it would define a bit better contract for aws-sdk clients.

kuhe commented 6 months ago

@aws-sdk/smithy-client re-exports everything from @smithy/smithy-client, so although it is deprecated, it will function in the same way, even on prototype identity if no package duplication exists.

In what way is middleware not working across the package migration of smithy-client?

rheidari commented 6 months ago

We cannot use @aws-sdk/smithy-client because it is marked as private beginning in 3.375.0

aBurmeseDev commented 5 months ago

Hi @workeitel - checking in here if you still have any other questions we can answer.

workeitel commented 5 months ago

The problem is not really resolved. @aws-sdk/smithy-client does not work anymore and instead all consumers need to move to @smithy/smithy-client. Similar for other classes. Hard-coding own types could help but as soon as the middleware needs to interact with the client the problem still persists.

For example a middleware accessing the retry strategy similar to:

const retry = await client.config.retryStrategy();
if (retry instanceof AdaptiveRetryStrategy) {
  ...

would again not work as AdaptiveRetryStrategy changed from @aws-sdk/util-retry to @smithy/util-retry.

I understand for a generic smithy client that's not possible but aws-sdk clients are more uniform. I understand you can't guarantee the types won't change between releases but at least the import parts should really stay as otherwise consumers always need a code change.

github-actions[bot] commented 4 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.