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

MIGRATION ISSUE: Unknown (403) error on POST policy condition violation (forbidden) #6597

Closed
4 of 5 tasks
yvele opened this issue Oct 29, 2024 · 4 comments
Closed
4 of 5 tasks
Labels
p2 This is a standard priority issue v2-v3-inconsistency Behavior has changed from v2 to v3, or feature is missing altogether

Comments

@yvele
Copy link

yvele commented Oct 29, 2024

Pre-Migration Checklist

Which JavaScript Runtime is this issue in?

Node.js (includes AWS Lambda)

AWS Lambda Usage

  • Yes, my application is running on AWS Lambda.
  • No, my application is not running on AWS Lambda.

Describe the Migration Issue

When sending an HeadObjectCommand on an existing S3 object that has been uploaded without respecting POST policy conditions (size out of range in my particular case) I get a 403 "UnknownError" with AWS SDK v3:

{
    "name": "403",
    "$fault": "client",
    "$metadata": {
        "httpStatusCode": 403,
        "requestId": "XXXXX",
        "extendedRequestId": "XXXXX/XXXXX/XXXXX=",
        "attempts": 1,
        "totalRetryDelay": 0
    },
    "message": "UnknownError"
}

With AWS SDK v2 the error code was "Forbidden".

Code Comparison

AWS SDK v2

let head;
try {
  head = await s3.headObject({
    Bucket : "bucketName",
    Key : "key"
  }).promise();
} catch (err) {
  if (err.code === "Forbidden") {
    // Catched!
  }
  throw err;
}

AWS SDK v3

const headObjectCommand = new HeadObjectCommand({
  Bucket : "bucketName",
  Key : "key"
});
let head;
try {
  head = await s3Client.send(headObjectCommand);
} catch (err) {
  if (err.name === "403" && err.message === "UnknownError") {
    // Catched!
  }
  throw err;
}

Observed Differences/Errors

I'm using the AWS SDK v3 and typically catch errors based on their specific types, such as if (err instanceof NoSuchKey).

However, when handling POST policy condition violations, I encounter an ambiguous "unknown error" rather than a specific Forbidden error as expected. This lack of clarity makes it difficult to diagnose and handle POST policy condition issues effectively.

Additional Context

@aws-sdk/client-s3 v3.679.0

@yvele yvele added needs-triage This issue or PR still needs to be triaged. v2-v3-inconsistency Behavior has changed from v2 to v3, or feature is missing altogether labels Oct 29, 2024
@RanVaknin
Copy link
Contributor

RanVaknin commented Oct 31, 2024

Hi,

I think it has to do with JS v2 having a customization around it.
If I had to guess, it's happening here: https://github.com/aws/aws-sdk-js/blob/5d0e38adbbc1a3fd6e6bf7c48cd7e209e9eb0b5f/lib/services/s3.js#L713

S3 uses a RESTXML protocol and sends response information in the body of the request. Since HEAD operations like headObject do not have a body, the SDK cannot "map" the response body to a concrete error type (because there is no body)

In v2, my guess is that this was introduced as a customization to mimic what a server response would have been (403 will almost always map to some sort of a Forbidden error). Since the server never responds with Forbidden, the SDK introduces a synthetic error that only looks like a concrete typed error, but its not.

in v3, the lack of concrete error sent (again because there is no body) means that the SDK has to fallback to a generic error (UnknownError).

This is intentional as in the newer SDKs the teams are moving away from handwritten customizations to better reflect the actual I/O sent and received from the various APIs.

FWIW for S3's head operations:

//v3 
if (err.name === "403" && err.message === "UnknownError") 

// is the same as

//v2
if (err.code === "Forbidden") {

You can verify that by logging the raw response sent from the server in your v3 code:

s3Client.middlewareStack.add(next => async (args) => {
 console.log(args.request)
 const response = await next(args);
 console.log(response);
 return response;
}, {step: 'finalizeRequest'})

This will show you the raw response as it is sent from the server and you can confirm whether the server sends back Forbidden in the response.

Thanks,
Ran~

@aBurmeseDev aBurmeseDev added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Nov 1, 2024
@yvele
Copy link
Author

yvele commented Nov 2, 2024

@RanVaknin Thank you you answer.
From what I understand, this is the intentional v3 behavior, correct?

403 will almost always map to some sort of a Forbidden error

Is err.message === "UnknownError" necessary or I could I simply check err.name === "403" ?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Nov 3, 2024
@RanVaknin
Copy link
Contributor

Hi @yvele ,

The intentional part here is to drop synthetic errors as they do not reflect the actual response from the server.

Is err.message === "UnknownError" necessary or I could I simply check err.name === "403" ?

Any error raised from a head operation will result in an UnknownError, so if you are writing error handling code for head operations you can simply use the status 403 to deduct its a Forbidden error.

This is unfortunately just a limitation of S3. General error handling best practices don't apply here since in both v2 and v3's case, we are inferring what the error is based on metadata alone. The only difference here between v2 and v3 is semantics.

Thanks,
Ran~

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p2 This is a standard priority issue v2-v3-inconsistency Behavior has changed from v2 to v3, or feature is missing altogether
Projects
None yet
Development

No branches or pull requests

3 participants