feat!: upgrade AWS SDK to v3 #83

gbryan · 2022-08-19T03:26:06Z

Ticket: https://app.asana.com/0/1200444444072189/1202815880040240

We will not yet merge this because we cannot upgrade Beyonce's AWS SDK to v3 without also upgrading Vault's. Timeboxing Vault's AWS SDK to v3 to 30 minutes indicated that it isn't as quick enough a task that we can take it on right now. We can pick that up (and also merge + test E2E this PR) in the future within one of our teams' tech debt budgets.

This PR upgrades Beyonce from v2 to v3 of the AWS SDK, which involves breaking changes: clients need to instantiate Beyonce with a DynamoDB instance from @aws-sdk/client-dynamodb v3 instead of aws-sdk v2.

Reviewing commit-by-commit will be easiest.

The original motivation for this upgrade was to experiment with the new error responses from DynamoDB when encountering a condition check failure while executing a transaction, to see if the new error messages provide enough detail to determine which condition check failed (so we could distinguish the reason(s) in the case that there were to be multiple condition checks). Related ticket. It turns out that even the new v3 error responses do not contain such detail. For example, here's an error:

    TransactionCanceledException: Transaction cancelled, please refer cancellation reasons for specific reasons [None, ConditionalCheckFailed]
        at deserializeAws_json1_0TransactionCanceledExceptionResponse (/Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/client-dynamodb/dist-cjs/protocols/Aws_json1_0.js:3072:23)
        at deserializeAws_json1_0TransactWriteItemsCommandError (/Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/client-dynamodb/dist-cjs/protocols/Aws_json1_0.js:2380:25)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at /Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/middleware-serde/dist-cjs/deserializerMiddleware.js:7:24
        at /Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/middleware-signing/dist-cjs/middleware.js:11:20
        at StandardRetryStrategy.retry (/Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/middleware-retry/dist-cjs/StandardRetryStrategy.js:51:46)
        at /Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:6:22
        at /Users/bryangaston/workspace/gingerio/beyonce/node_modules/@aws-sdk/lib-dynamodb/dist-cjs/commands/TransactWriteCommand.js:36:26 {
      '$fault': 'client',
      '$metadata': {
        httpStatusCode: 400,
        requestId: 'e3e60378-8cbf-44b4-9a8c-15869adec843',
        extendedRequestId: undefined,
        cfId: undefined,
        attempts: 1,
        totalRetryDelay: 0
      },
      CancellationReasons: [
        { Code: 'None', Item: undefined, Message: undefined },
        {
          Code: 'ConditionalCheckFailed',
          Item: undefined,
          Message: 'The conditional request failed'
        }
      ],
      __type: 'com.amazonaws.dynamodb.v20120810#TransactionCanceledException'
    }

gbryan · 2022-08-19T03:27:49Z

Dockerfile

@@ -1,14 +1,14 @@
-# Needed for DynamoDB Local, which is used for unit tests
-FROM openjdk:15-alpine3.11
+# Java JDK is needed for DynamoDB Local, which is used for unit tests


To get AWS SDK v3 support for the local DynamoDB, we have to upgrade jest-dynamodb, which necessitates upgrading to a newer version of Nodejs, which necessitates switching from the deprecated base Docker image we were using to this one.

gbryan · 2022-08-19T03:29:05Z

jest-dynamodb-config.js

@@ -12,8 +12,10 @@ module.exports = async () => {
    port,
    clientConfig: {
      endpoint,
-      sslEnabled: false,
-      region: "local",
+      credentials: {


In my testing, I got prompts to fetch new AWS SSO credentials unless I provided dummy credentials for the local DynamoDB.

gbryan · 2022-08-19T03:29:49Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "@ginger.io/beyonce",
-  "version": "0.0.66",
+  "version": "0.0.67",


This PR has breaking changes, but since we haven't hit version 1, I'm assuming we're ok with just continuing to bump the version like this instead of going to version 1.0.0 now. LMK if you'd prefer otherwise!

gbryan · 2022-08-19T03:30:32Z

package.json

@@ -12,8 +12,9 @@
    "./src"
  ],
  "dependencies": {
+    "@aws-sdk/client-dynamodb": "3.113.0",


I'm pinning to the same versions that @shelf/jest-dynamodb uses because I was encountering errors with that library before doing so.

gbryan · 2022-08-19T03:31:44Z

src/main/dynamo/Beyonce.ts

  private jayz?: JayZ
  private consistentReads: boolean

  constructor(private table: Table<string, string>, dynamo: DynamoDB, options: Options = {}) {
-    this.client = new DynamoDB.DocumentClient({ service: dynamo })
+    this.client = DynamoDBDocumentClient.from(dynamo, {
+      marshallOptions: { removeUndefinedValues: true }


Attempting to write keys with undefined values results in errors unless specifying this option.

gbryan · 2022-08-19T03:33:58Z

src/main/dynamo/operations/batchWriteItems.ts

@@ -29,6 +30,8 @@ export async function batchWriteItems<T extends TaggedModel>(
  deleteItems.forEach((key) => {
    requests.push({
      DeleteRequest: {
+        // @ts-ignore The type is incorrect for `Key`. It expects Record<string, AttributeValue>, but since we're using
+        //  a DynamoDBDocumentClient, it needs to be Record<string, string> instead.


Writes fail if I pass an object in the format Record<string, AttributeValue>. The type in the underlying library is wrong.

gbryan · 2022-08-19T03:34:16Z

src/main/dynamo/operations/batchWriteItems.ts

  const unprocessedPuts: T[] = []
  const unprocessedDeleteKeys = new UnprocessedKeyCollector(table, deleteItems)

  if (results.UnprocessedItems && results.UnprocessedItems[tableName]) {
    results.UnprocessedItems[tableName].forEach(({ PutRequest, DeleteRequest }) => {
      if (PutRequest) {
        unprocessedPuts.push(PutRequest.Item as T)
-      } else if (DeleteRequest) {
+      } else if (DeleteRequest && DeleteRequest.Key) {


DeleteRequest.Key is now undefined.

gbryan · 2022-08-19T03:34:36Z

src/main/dynamo/operations/transactWriteItems.ts

@@ -33,6 +34,8 @@ export async function transactWriteItems<T extends TaggedModel>(params: Transact
    requests.push({
      Delete: {
        TableName: table.tableName,
+        // @ts-ignore The type is incorrect for `Key`. It expects Record<string, AttributeValue>, but since we're using


Same here: the type in the underlying lib is wrong.

gbryan · 2022-08-19T03:35:23Z

src/main/dynamo/operations/transactWriteItems.ts

-    } else {
-      throw e
+    if (e && e.CancellationReasons) {
+      throw new Error(`${e.message}. Cancellation Reasons: ${JSON.stringify(e.CancellationReasons)}`)


It's nice that we can skip all the parsing from the httpResponse now.

gbryan · 2022-08-19T03:36:14Z

src/main/dynamo/util.ts

  }
 }
+
+/**
+ * The DynamoDB v3 lib returns items with values that are UInt8Array instead of Buffer, so this function


The response format appears to have changed from Buffer to UInt8Array, so we're converting to keep the expected format for clients.

gbryan · 2022-08-19T03:36:53Z

src/test/dynamo/Beyonce.test.ts

-      ConsistentRead: true
-    })
+    expect(mockGet).toHaveBeenCalledWith(
+      expect.objectContaining({


We use objectContaining now since there are various other properties on the argument passed to send() now.

gbryan added 5 commits August 18, 2022 20:18

feat: upgrade dynamodb lib

cc41566

upgrade jest-dynamodb

c59f07d

convert UInt8Array to Buffer on returned items

c520dc9

Upgrade Node to support new version of jest-dynamodb

3030463

bump version

472d2e9

gbryan self-assigned this Aug 19, 2022

gbryan commented Aug 19, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: upgrade AWS SDK to v3 #83

feat!: upgrade AWS SDK to v3 #83

gbryan commented Aug 19, 2022 •

edited

Loading

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

gbryan Aug 19, 2022

feat!: upgrade AWS SDK to v3 #83

Are you sure you want to change the base?

feat!: upgrade AWS SDK to v3 #83

Conversation

gbryan commented Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbryan commented Aug 19, 2022 •

edited

Loading