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

GetObjectCommand throws Error named "304" instead of "NotModified" for If-None-Match with matching ETag #5860

Open
3 tasks done
DougReeder opened this issue Mar 6, 2024 · 4 comments
Labels
bug This issue is a bug. p2 This is a standard priority issue queued This issues is on the AWS team's backlog

Comments

@DougReeder
Copy link

DougReeder commented Mar 6, 2024

Checkboxes for prior research

Describe the bug

When I use GetObjectCommand with the IfNoneMatch parameter set to the ETag that matches the current value of the object, I get an unexpected error:

{
  "name": "304",
  "$fault": "client",
  "$metadata": {
    "httpStatusCode": 304,
    "requestId": "txf0dab84e1901420c995fe-0065e8c28e",
    "extendedRequestId": "txf0dab84e1901420c995fe-0065e8c28e",
    "attempts": 1,
    "totalRetryDelay": 0
  },
  "message": "UnknownError"
}

According to the docs it should return return a "304 Not Modified error". I can work around this oddity, but it would be nice to have this error named consistently with other.

SDK version number

@aws-sdk/package-name@3.523.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.10.0

Reproduction Steps

try { 
    let getParam = { Bucket: username, Key: s3Path, IfNoneMatch: condition.ETag };

  const { Body, ETag, ContentType, ContentLength } = await this.#S3Client.send(new GetObjectCommand(getParam));
    //...
} catch (err) {
    if (err.name === 'NotModified' ) {
        return { status: 304, readStream: null, contentType: null, contentLength: null, ETag: null    };
    } else {
        throw Object.assign(err, { status: 502 });
    }

Observed Behavior

This error is thrown:

{
  "name": "304",
  "$fault": "client",
  "$metadata": {
    "httpStatusCode": 304,
    "requestId": "txf0dab84e1901420c995fe-0065e8c28e",
    "extendedRequestId": "txf0dab84e1901420c995fe-0065e8c28e",
    "attempts": 1,
    "totalRetryDelay": 0
  },
  "message": "UnknownError"
}

Expected Behavior

An error with name "NotModified"

Possible Solution

double-check there is code to handle an HTTP status of 304

Additional Information/Context

I confess I have only run this against min.io and OpenIO - i assumed AWS S3 would work. I'm scheduled to test against AWS S3 soon.

@DougReeder DougReeder added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 6, 2024
@DougReeder
Copy link
Author

Using the IfMatch parameter set to an ETag that doesn't match, it throws an error with name "PreconditionFailed", as expected.

@kellertk
Copy link
Contributor

kellertk commented Mar 6, 2024

Reproduction:

  1. Put a file in S3.
C:\dev\jsv3-5860 ❯ aws s3 cp test.txt s3://imalittlebucketshortandstout/
upload: .\test.txt to s3://imalittlebucketshortandstout/test.txt
  1. Install the s3 client.
C:\dev\jsv3-5860 ❯ npm ls @aws-sdk/client-s3
jsv3-5860@1.0.0 C:\dev\jsv3-5860
└── @aws-sdk/client-s3@3.523.0
  1. Confirm object in bucket.
C:\dev\jsv3-5860 ❯ aws s3api list-objects --bucket imalittlebucketshortandstout
{
    "Contents": [
        {
            "Key": "test.txt",
            "LastModified": "2024-03-06T20:48:40+00:00",
            "ETag": "\"d41d8cd98f00b204e9800998ecf8427e\"",
            "Size": 0,
            "StorageClass": "STANDARD",
            "Owner": {
                "DisplayName": "kellertk+supportenabled",
                "ID": "e5c2e26c29682046cff3e89b8c82330125e5bb662eb618ee990989c1f58babe7"
            }
        }
    ],
    "RequestCharged": null
}
  1. Write some minimal repro code.
C:\dev\jsv3-5860 ❯ cat index.js
import { S3Client, GetObjectCommand } from '@aws-sdk/client-s3';

const client = new S3Client();
const param = { Bucket: 'imalittlebucketshortandstout',
                Key: 'test.txt',
                IfNoneMatch: 'd41d8cd98f00b204e9800998ecf8427e' };
try {
    const { Body, ETag, ContentType, ContentLength } = await client.send(new GetObjectCommand(param));
    console.log('Object retrieved', { ETag, ContentType, ContentLength });
    console.log('Body:', Body.toString());
    process.exit(0);
} catch (err) {
    if (err.name === 'NotModified') {
        console.log('Object not modified');
    } else {
        console.log('Error retrieving object', err);
    }
}
  1. Run test case. Looks like it fails in the way you reported.
C:\dev\jsv3-5860 ❯ node index.js
Error retrieving object 304: UnknownError
    at throwDefaultError (C:\dev\jsv3-5860\node_modules\@smithy\smithy-client\dist-cjs\index.js:838:20)
    at C:\dev\jsv3-5860\node_modules\@smithy\smithy-client\dist-cjs\index.js:847:5
    at de_CommandError (C:\dev\jsv3-5860\node_modules\@aws-sdk\client-s3\dist-cjs\index.js:4756:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async C:\dev\jsv3-5860\node_modules\@smithy\middleware-serde\dist-cjs\index.js:35:20
    at async C:\dev\jsv3-5860\node_modules\@aws-sdk\middleware-signing\dist-cjs\index.js:192:18
    at async C:\dev\jsv3-5860\node_modules\@smithy\middleware-retry\dist-cjs\index.js:320:38
    at async C:\dev\jsv3-5860\node_modules\@aws-sdk\middleware-flexible-checksums\dist-cjs\index.js:173:18
    at async C:\dev\jsv3-5860\node_modules\@aws-sdk\middleware-sdk-s3\dist-cjs\index.js:97:20
    at async C:\dev\jsv3-5860\node_modules\@aws-sdk\middleware-sdk-s3\dist-cjs\index.js:120:14 {
  '$fault': 'client',
  '$metadata': {
    httpStatusCode: 304,
    requestId: 'VSVZA69DKP98RYZN',
    extendedRequestId: 'Y3yAkxGzjzxbeB0/4uX0LIV+aCTePM1m32x6Vid99/tJPTNjw5YPrYHZfODyOw0bm6WwDVguWxM=',
    cfId: undefined,
    attempts: 1,
    totalRetryDelay: 0
  }
}
  1. Try with the CLI APIs to see what S3 is giving us back. Looks like they're doing 304 Not Modified.
C:\dev\jsv3-5860 ❯ aws s3api get-object --bucket imalittlebucketshortandstout --if-none-match d41d8cd98f00b204e9800998ecf8427e --key test.txt -

An error occurred (304) when calling the GetObject operation: Not Modified

I think this error is being modeled incorrectly, which is why you're seeing this. These errors are generated from the service's Smithy models. I'll make sure to mark this one as needing review with the team, but if you need to workaround this in the immediate term, check out this comment on how to plug in a custom error handler in the middleware stack.

@kellertk kellertk added needs-review This issue/pr needs review from an internal developer. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Mar 6, 2024
@aBurmeseDev aBurmeseDev added queued This issues is on the AWS team's backlog and removed needs-review This issue/pr needs review from an internal developer. labels Mar 6, 2024
@DougReeder
Copy link
Author

DougReeder commented Mar 6, 2024

Thanks! As the odd error does have 304 in it, I can work with that:

if (err.name === 'NotModified' || err.$metadata?.httpStatusCode === 304 || err.name === 304) {
  return { status: 304, readStream: null, contentType: null, contentLength: null, ETag: null };
}

It's good to know about custom error handlers.

@RanVaknin RanVaknin self-assigned this Mar 8, 2024
@RanVaknin
Copy link
Contributor

RanVaknin commented Mar 8, 2024

For whoever picks this to work on, here is some more details.

The SDK doesn't deserialize the NotModified error since it is not defined in the API model. The server does return it under the reason field if you log the error.$response:

// rest of the code
} catch (err) {
  console.log(err.$response)
}
/*
HttpResponse {
  statusCode: 304,
  reason: 'Not Modified',
  headers: {
    'x-amz-id-2': 'REDACTED',
    'x-amz-request-id': 'REDACTED',
    date: 'Fri, 08 Mar 2024 18:55:39 GMT',
    'last-modified': 'Fri, 08 Mar 2024 18:50:48 GMT',
    etag: '"c31ea01ca12b5558b6503a8143cdb98c"',
    'x-amz-version-id': 'WMcd2TdyBPRGGa2YdLXIHk2i0HqCZGU8',
    server: 'AmazonS3'
  },
*/

The Go SDK does have support for this even though its not specified in the model, since their error parsing is more deterministic.

@RanVaknin RanVaknin removed their assignment Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

4 participants