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

fix(middleware-flexible-checksums): skip checksum validation if CRC64NVME dependency is absent #6835

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Jan 21, 2025

Issue

Fixes: #6822

Description

Skips checksum validation if CRC64NVME dependency is absent

Testing

Unit testing

Perform putObject with CRC64NVME dependency

// putObject.mjs
import { S3, ChecksumAlgorithm } from "@aws-sdk/client-s3";
import "@aws-sdk/crc64-nvme-crt";

const client = new S3();

const Bucket = "test-flexible-checksums-v2"; // Change to your bucket name.
const Key = "foo";
const Body = "bar";

await client.putObject({ Bucket, Key, Body, ChecksumAlgorithm: ChecksumAlgorithm.CRC64NVME });

getObject without CRC64NVME dependency

// getObject.mjs
import { S3 } from "../aws-sdk-js-v3/clients/client-s3/dist-cjs/index.js";

const client = new S3({ logger: console });

const Bucket = "test-flexible-checksums-v2";
const Key = "foo";

await client.getObject({ Bucket, Key });
Before

Error is thrown

node:internal/modules/run_main:122
    triggerUncaughtException(
    ^

Error: Please check whether you have installed the "@aws-sdk/crc64-nvme-crt" package explicitly. 
You must also register the package by calling [require("@aws-sdk/crc64-nvme-crt");] or an ESM equivalent such as [import "@aws-sdk/crc64-nvme-crt";]. 
For more information please go to https://github.com/aws/aws-sdk-js-v3#functionality-requiring-aws-common-runtime-crt
    at selectChecksumAlgorithmFunction (/Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-flexible-checksums/dist-cjs/index.js:209:15)
    at validateChecksumFromResponse (/Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-flexible-checksums/dist-cjs/index.js:411:35)
    at /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-flexible-checksums/dist-cjs/index.js:466:11
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/index.js:174:20
    at async /Users/trivikr/workspace/test/node_modules/@smithy/middleware-serde/dist-cjs/index.js:33:24
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/index.js:483:18
    at async /Users/trivikr/workspace/test/node_modules/@smithy/middleware-retry/dist-cjs/index.js:321:38
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/index.js:109:22
    at async /Users/trivikr/workspace/test/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/index.js:136:14 {
  '$metadata': { attempts: 1, totalRetryDelay: 0 }
}
After

No error is thrown, but a warning is emitted and validation is skipped.

...
Skipping CRC64NVME checksum validation: Please check whether you have installed the "@aws-sdk/crc64-nvme-crt" package explicitly. 
You must also register the package by calling [require("@aws-sdk/crc64-nvme-crt");] or an ESM equivalent such as [import "@aws-sdk/crc64-nvme-crt";]. 
For more information please go to https://github.com/aws/aws-sdk-js-v3#functionality-requiring-aws-common-runtime-crt
...

getObject with CRC64NVME dependency

// getObject.mjs
import { S3 } from "../aws-sdk-js-v3/clients/client-s3/dist-cjs/index.js";
import "../aws-sdk-js-v3/packages/crc64-nvme-crt/dist-cjs/index.js";

const client = new S3({ logger: console });

const Bucket = "test-flexible-checksums-v2";
const Key = "foo";

await client.getObject({ Bucket, Key });

No warning is emitted, and validation is performed.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@trivikr trivikr marked this pull request as ready for review January 21, 2025 06:03
@trivikr trivikr requested a review from a team as a code owner January 21, 2025 06:03
checksumAlgorithmFn = selectChecksumAlgorithmFunction(algorithm as ChecksumAlgorithm, config);
} catch (error) {
if (algorithm === ChecksumAlgorithm.CRC64NVME) {
logger?.warn(`Skipping ${ChecksumAlgorithm.CRC64NVME} checksum validation: ${error.message}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

currently, the only possible way to enter this branch is due to the thrown missing dependency error, but should this perhaps be more explicit?

method 1:

if (String(e).includes("@aws-sdk/crc64-nvme-crt")) {
  ...
  continue;
}

method 2:

// throw site
const error = new Error("...");
error.missingOptionalDependency = true;
throw error;
// catch site
if (error.missingOptionalDependency) {
  ...
  continue;
}

@trivikr trivikr merged commit 7dbc1c9 into aws:main Jan 21, 2025
4 checks passed
@trivikr trivikr deleted the skip-crc64nvme-validation branch January 21, 2025 16:10
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.

Next.JS 14.2.23 - Importing @aws-sdk/crc64-nvme-crt" as required but fails to fetch S3 objects.
3 participants