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: S3 HeadObject NotFound error never being returned #2139

Merged
merged 2 commits into from Jun 12, 2023

Conversation

aajtodd
Copy link
Contributor

@aajtodd aajtodd commented May 25, 2023

Issues:

fixes: #2084

Description:

Removes S3 HeadObject customization that stripped the model of all errors for that operation. It's unclear why we decided to adopt this behavior. The code references aws/aws-sdk-go#1208 as justification but the NoSuchKey error in question is not what is modeled. Both HeadBucket and HeadObject only list NotFound as an error.

This also would appear to fix the ObjectExists and ObjectNotExists waiters.

Backwards compatibility concerns

The NotFound error was never possible before this so if customers have defined code paths for it they may now trigger them. The NotFound error type is still an APIError though and any code path that implemented a work around to check the ErrorCode() directly will still work since NotFound implements APIError

e.g.

	var apiErr smithy.APIError
	if errors.As(err, &apiErr) && apiErr.ErrorCode() == "NotFound" { ... }  // still works fine with `NotFound` error

Testing

package main

import (
	"context"
	"errors"
	"fmt"
	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/types"
	"log"
)

func main() {
	ctx := context.Background()
	if err := issue(ctx); err != nil {
		log.Fatal(err)
	}
}

func issue(ctx context.Context) error {
	cfg, err := config.LoadDefaultConfig(
		ctx,
		config.WithClientLogMode(aws.LogRequestWithBody|aws.LogResponseWithBody),
		config.WithSharedConfigProfile("dev-admin-west"),
	)

	if err != nil {
		return err
	}
	client := s3.NewFromConfig(cfg)

	bucketResp, err := client.HeadBucket(ctx, &s3.HeadBucketInput{
		Bucket: aws.String("does-not-exist-foobar-go-test"),
	})
	fmt.Printf("resp: %v, err: %v\n", bucketResp, err)

	var notFoundErr *types.NotFound
	if errors.As(err, &notFoundErr) {
		fmt.Printf("HeadBucket returned not found: %v\n", notFoundErr)
	} else {
		fmt.Println("HeadBucket did not return not found")
	}

	notFoundErr = nil
	bucket := "<my-bucket>"
	objResp, err := client.HeadObject(ctx, &s3.HeadObjectInput{
		Bucket: aws.String(bucket),
		Key:    aws.String("does-not-exist-key"),
	})
	fmt.Printf("resp: %v, err: %v\n", objResp, err)

	if errors.As(err, &notFoundErr) {
		fmt.Printf("HeadObject returned not found: %v\n", notFoundErr)
	} else {
		fmt.Println("HeadObject did not return not found")
	}
	return nil
}

Output Before

SDK 2023/05/25 12:41:35 DEBUG Response
HTTP/1.1 404 Not Found
Connection: close
Content-Type: application/xml
Date: Thu, 25 May 2023 16:41:34 GMT
Server: AmazonS3

resp: <nil>, err: operation error S3: HeadBucket, https response error StatusCode: 404, RequestID: K5FRB1PK1V6F4TX5, HostID: 1234=, NotFound: 
HeadBucket returned not found: NotFound: 


SDK 2023/05/25 12:41:36 DEBUG Response
HTTP/1.1 404 Not Found
Connection: close
Content-Type: application/xml
Date: Thu, 25 May 2023 16:41:35 GMT
Server: AmazonS3

resp: <nil>, err: operation error S3: HeadObject, https response error StatusCode: 404, RequestID: Y7PBZF6SNBBCPFJG, HostID: 1234=, api error NotFound: Not Found
HeadObject did not return not found

Output After

SDK 2023/05/25 13:12:46 DEBUG Response
HTTP/1.1 404 Not Found
Connection: close
Content-Type: application/xml
Date: Thu, 25 May 2023 17:12:46 GMT
Server: AmazonS3
X-Amz-Id-2: 4AoYTha5idyFA6zjSODv4ymcw1lT4cGedEJ8hT8XUY9w4+A3DYpPzxHtwAb7X47nQ1710wpRzks=
X-Amz-Request-Id: MG3KCK7QJD3YYEKT

resp: <nil>, err: operation error S3: HeadBucket, https response error StatusCode: 404, RequestID: MG3KCK7QJD3YYEKT, HostID: 4AoYTha5idyFA6zjSODv4ymcw1lT4cGedEJ8hT8XUY9w4+A3DYpPzxHtwAb7X47nQ1710wpRzks=, NotFound: 
HeadBucket returned not found: NotFound: 

SDK 2023/05/25 13:12:47 DEBUG Response
HTTP/1.1 404 Not Found
Connection: close
Content-Type: application/xml
Date: Thu, 25 May 2023 17:12:46 GMT
Server: AmazonS3

resp: <nil>, err: operation error S3: HeadObject, https response error StatusCode: 404, RequestID: FR98XVN62NFF1FPS, HostID: 1234=, NotFound: 
HeadObject returned not found: NotFound: 

Appendix

Model

Links to S3 model never work because it's too large, pasted here for convenience

        "com.amazonaws.s3#HeadBucket": {
            "type": "operation",
            "input": {
                "target": "com.amazonaws.s3#HeadBucketRequest"
            },
            "output": {
                "target": "smithy.api#Unit"
            },
            "errors": [
                {
                    "target": "com.amazonaws.s3#NotFound"
                }
            ],



        "com.amazonaws.s3#HeadObject": {
            "type": "operation",
            "input": {
                "target": "com.amazonaws.s3#HeadObjectRequest"
            },
            "output": {
                "target": "com.amazonaws.s3#HeadObjectOutput"
            },
            "errors": [
                {
                    "target": "com.amazonaws.s3#NotFound"
                }
            ],

@aajtodd aajtodd requested a review from a team as a code owner May 25, 2023 17:36
@aajtodd aajtodd merged commit ba5a08f into main Jun 12, 2023
19 checks passed
@aajtodd aajtodd deleted the fix-head-obj-not-found branch June 12, 2023 20:27
{
"id": "d9de6f3d-d7c1-46c9-9666-48ecda1fa2a8",
"type": "bugfix",
"description": "Fix HeadObject to return types.Nound when an object does not exist. Fixes [2084](https://github.com/aws/aws-sdk-go-v2/issues/2084)",

Choose a reason for hiding this comment

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

types.Nound 🤔 should be types.NotFound?

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.

s3 HeadObject still doesn't return types.NotFound error
4 participants