effect-aws icon indicating copy to clipboard operation
effect-aws copied to clipboard

feat: improve client generation

Open godu opened this issue 1 year ago • 5 comments

  • Regenerates clients : iam, ec2, elasticache, sns.
  • Improves codegen with some ideas from https://github.com/floydspace/effect-aws/pull/46
    • Exports exceptions with the same name as the original package
    mport type {
     ...
       UnrecognizedPublicKeyEncodingException,
       UnrecognizedPublicKeyEncodingException as SdkUnrecognizedPublicKeyEncodingException,
     ...
     from "@aws-sdk/client-iam";
     export type `ConcurrentModificationError` =
       TaggedException<ConcurrentModificationException>;
     export type ConcurrentModificationException =
       TaggedException<SdkConcurrentModificationException>;
    
    • Uses all operator shapes from model instead of only those of the service shape.
    • Setup default values of client generation.

godu avatar Apr 18 '24 15:04 godu

I've already experienced an error when the aws-sdk version that is used to generate the client was higher then the the aws-sdk used in a project. Maybe it's good to pin the version of the aws-sdk, or at least be minimum the version used to generate?

e.g. package.json of client-cloudwatch-events

    "@aws-sdk/client-cloudwatch-events": "^3.556.0",
    "@aws-sdk/types": "^3.556.0"

joepjoosten avatar Apr 19 '24 08:04 joepjoosten

Hi, i'm using my own generated aws clients based on the code from the effect-aws repo (my pullrequest). And i've noticed that the tree shaken esm build is still rather large. This is because the generated service contains a:

const commands = {
  all commands...
}

Which can't be tree shaken. So it also will include all imports of the commands... This is really large, even if you only use one command in your function. I've been tinkering with the implementation, and i think i found a solution, which is maybe also more like the methods and functions in the effect library itself:

This is an example of the implementation for the SSM client:

export const updateServiceSetting: (
  args: UpdateServiceSettingCommandInput,
  options?: __HttpHandlerOptions,
) => Effect.Effect<
  UpdateServiceSettingResult,
  | SdkError
  | InternalServerError
  | ServiceSettingNotFound
  | TooManyUpdates,
  SSMClientInstance
> = typedErrors(UpdateServiceSettingCommand);

function typedErrors<R, E, T extends new (...args: any) => any>(command: T) {
  return (args: ConstructorParameters<T>[0], options?: __HttpHandlerOptions) => Effect.gen(function* (_) {
    const client = yield* _(SSMClientInstance);
    return yield* _(Effect.tryPromise({
        try: () => client.send(new command(args), options),
        catch: (e) => {
          if (e instanceof SdkSSMServiceException) {
            const ServiceException = Data.tagged<
              TaggedException<SdkSSMServiceException>
            >(e.name);
  
            return ServiceException({
              ...e,
              message: e.message,
              stack: e.stack,
            });
          }
          if (e instanceof Error) {
            return SdkError({
              ...e,
              name: "SdkError",
              message: e.message,
              stack: e.stack,
            });
          }
          throw e;
        },
      }) as Effect.Effect<R, E>)
  });
}

What is your opinion?

joepjoosten avatar Apr 30 '24 10:04 joepjoosten

You're right, I think we should rework it to be more tree-shakable. I don't like your proposal because you embed the @aws-sdk binding in the effect logic. I think, like in hexagone architecture's ports do, we should scope this part in the service.

I tried something here (the typing could be improved). I create a Service for each Command.

It'll be easier to mock it.

    const listRegionsServiceMock = {
      listRegions: vi.fn().mockImplementation(() => Effect.succeed({})),
    };

    const result = await pipe(
      program,
      Effect.provide(Layer.succeed(ListRegionsService, listRegionsServiceMock)),
      Effect.runPromiseExit,
    );

    expect(result).toEqual(Exit.succeed({}));
    expect(listRegionsServiceMock.listRegions).toHaveBeenNthCalledWith(1, {});

But the metafile still show me the all @aws-sdk's commands and the @effect-aws's unused services.

esbuild --minify --format=esm --target=es2022 --bundle --platform=node test/input.ts --metafile=dist/meta.json --outfile=dist/output.js --tsconfig=tsconfig.esm.json

meta.json

Perhaps I miss something;

godu avatar May 02 '24 13:05 godu

I've fixed this by creating a separate file per command. This will fix the issue with unused imports. The only duplication left is something that needs to be solved in esbuild: https://github.com/evanw/esbuild/issues/475

joepjoosten avatar May 04 '24 19:05 joepjoosten

I don't like the approach to have a service per command. Then you need to create a layer for all commands used in the underlying effects. This will reduce re-usability, and creating a default layer with all services is not tree shake able again...

You are right that it's much nicer when creating mocks for a service. But i think using something like aws-sdk-client-mock can help creating mocks. I'm currently trying this out, how this would look like.

joepjoosten avatar May 04 '24 19:05 joepjoosten