Skip to content
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

feat: encoders accept strings #1176

Merged
merged 3 commits into from
Mar 7, 2024
Merged

feat: encoders accept strings #1176

merged 3 commits into from
Mar 7, 2024

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented Feb 29, 2024

Clients currently have two internal fields of type Encoder, currently Uint8Array => string.

These have been updated to be Uint8Array | string => string, to allow direct conversion of utf8 strings to base64, rather than requiring a Uint8Array input.

The Encoder type has been loosened, since it is public.

related issues:
aws/aws-sdk-js-v3#5745
aws/aws-sdk-js-v3#5240

@kuhe kuhe requested review from a team as code owners February 29, 2024 18:10
@kuhe kuhe requested a review from sugmanue February 29, 2024 18:10
@kuhe kuhe force-pushed the feat/encoding branch 3 times, most recently from 0fdd0bc to 2d6e55f Compare February 29, 2024 19:14
@kuhe
Copy link
Contributor Author

kuhe commented Feb 29, 2024

This will allow users to send strings as input for fields that otherwise require Uint8Array, which can be cumbersome to initialize.

Though it is as easy as writing Buffer.from('abcd'), discovering this is somewhat obscure because it's not obvious that Buffer is a Uint8Array.

@kuhe
Copy link
Contributor Author

kuhe commented Mar 5, 2024

I verified this using SES

import * as sesClientModule from "@aws-sdk/client-ses";
import nodemailer from "nodemailer";

export const sendEmailWithAttachments = (from = "@email", to = "@email") => {
  const ses = new sesClientModule.SESClient({
    region: "us-west-2",
  });

  ses.middlewareStack.add(
    (next, context) => async (args: any) => {
      console.log("AWS SDK context", context.clientName, context.commandName);

      args.input.RawMessage.Data = args.input.RawMessage.Data.toString();
      console.log("AWS SDK request input", args.input);

      console.log("AWS SDK raw request", args.request);
      const result = await next(args);
    //   console.log("AWS SDK raw response", result.response);
      console.log("AWS SDK response metadata:", result.output.$metadata);
      // console.log("AWS SDK request output:", result.output);
      return result;
    },
    {
      name: "MyMiddleware",
      step: "build",
      override: true,
    }
  );

  const transporter = nodemailer.createTransport({
    SES: { ses, aws: sesClientModule },
  });

  return new Promise((resolve, reject) => {
    transporter.sendMail(
      {
        from,
        to,
        subject: "using string input for RawMessage",
        text: "using string input for RawMessage",
        attachments: [{ content: "Hello World!", filename: "hello.txt" }],
      },
      (err, info) => {
        if (err) {
          reject(err);
        } else {
          resolve(info);
        }
      }
    );
  });
};

sendEmailWithAttachments();

Copy link
Contributor

@syall syall left a comment

Choose a reason for hiding this comment

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

For inputs that take Uint8Array | string, can we do a runtime check if it's not one of these types to restrict the types more?

I see typeof xxx === "string" is used, but can we use instanceof Uint8Array checks?

@kuhe kuhe merged commit 43f3e1e into smithy-lang:main Mar 7, 2024
7 checks passed
@kuhe kuhe deleted the feat/encoding branch March 7, 2024 15:57
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.

None yet

2 participants