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

Unable to check existence in Laravel AWS s3 v3 #1604

Closed
arunkumar-mk19 opened this issue Dec 1, 2022 · 8 comments
Closed

Unable to check existence in Laravel AWS s3 v3 #1604

arunkumar-mk19 opened this issue Dec 1, 2022 · 8 comments

Comments

@arunkumar-mk19
Copy link

arunkumar-mk19 commented Dec 1, 2022

Bug Report

Q A
Flysystem Version 3.0
Adapter Name Aws S3 (v3)
Adapter version ^3.0

Summary

For checking file deleted success status. I check it with if file exists. Instead of returning false i am getting unable to check error.
"Unable to check existence for: uploads/file/lesson_zip/2-520123-Lesson_2_Pronouns-Project_reference.pdf on line 19 in file /var/www/html/vendor/league/flysystem/src/UnableToCheckExistence.php"

How to reproduce

@frankdejonge
Copy link
Member

Can you fill out specific version? and what S3 provider do you use? Do you have any special configuration? I have very little to debug your issue right now 😅

@mashb1t
Copy link

mashb1t commented Dec 2, 2022

+1, we're also experiencing this issue in Laravel 9.41 with league/flysystem-aws-s3-v3 3.10.3

@frankdejonge
Copy link
Member

Can you share the full stack trace and error message?

There is a test case for checking file existence after deletion and that passes for S3 in CI, which runs against AWS S3: https://github.com/thephpleague/flysystem/blob/3.x/src/AdapterTestUtilities/FilesystemAdapterTestCase.php#L276

@frankdejonge
Copy link
Member

I'm closing this for now, re-opening when there is more info.

@hoss-ps
Copy link

hoss-ps commented Dec 5, 2022

It's necessary to set the following permissions for the bucket otherwise most probably receive the error.

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "s3:ListBucket",
                "s3:GetObject",
                "s3:DeleteObject",
                "s3:GetObjectAcl",
                "s3:PutObjectAcl",
                "s3:PutObject"
            ],
            "Resource": [
                "arn:aws:s3:::your-bucket-name",
                "arn:aws:s3:::your-bucket-name/*"
            ]
        }
    ]
}

@deleugpn
Copy link

I'm getting this error now and I will have to be doing some shenanigans to debug/figure it out. But in case @frankdejonge wants to improve the issue even without more information available, I would say the root cause comes from here:

UnableToCheckFileExistence

which inherits this method:

    public static function forLocation(string $path, Throwable $exception = null): static
    {
        return new static("Unable to check existence for: {$path}", 0, $exception);
    }

and is called from here:

    public function fileExists(string $path): bool
    {
        try {
            return $this->client->doesObjectExist($this->bucket, $this->prefixer->prefixPath($path), $this->options);
        } catch (Throwable $exception) {
            throw UnableToCheckFileExistence::forLocation($path, $exception);
        }
    }

What essentially happens here is that there is an issue with the S3 configuration. The AWS S3 Client is giving us an exception about said issue. Flysystem is hiding that issue from us by wrapping it up on it's own exception. Laravel reporting system will report the exception UnableToCheckFileExistence which has no information about why S3 failed since Flysystem prevented the information from getting to us.

Some questions that we, users, have when we see this issue are:

  • What is the S3 Class that originated the error?
  • What is the S3 error message?

I understand that Flysystem is putting the previousException on it's own wrapper exception, but what that essentially means is that now I have to write custom code such as the following:

try {
    // ...
} catch (League\Flysystem\UnableToCheckExistence $e) {
    // Since Flysystem hid away the information I need, I need to specifically catch this exception so that I can access the original error:
    $this->exception->report($e->getPrevious());
}

All in all, what this means is that in the ecosystem of Laravel Exception Reporting and S3 Provider, Flysystem is hiding away crucial information from it's users and making us have to write custom code to handle Flysystem Exception just so that we can know what really happened.

@deleugpn
Copy link

Coming back here to add more info. Apparently the check for existing files never worked (aws/aws-sdk-php#2445) and always returned true. With the release of 3.12.1, this has been fixed by using the v2 method: https://github.com/thephpleague/flysystem-aws-s3-v3/blob/3.x/AwsS3V3Adapter.php#L118. Now something that failed to work properly because AWS SDK was never failing now started to work properly, which showed a slight complication on how Flysystem catches and hides the exception from users. The end result is that after doing composer update and releasing, we started getting errors because AWS SDK is throwing an exception, Flysystem is catching that exception and throwing it's own exception and the code no longer flows how we expected and we can't see why not.

@deleugpn
Copy link

my final findings can be read here: laravel/framework#45639

Conclusion is that there is an ambiguous 403 Forbidden thrown by AWS and we're left in a state where we can't reliably determine whether to return false or to throw exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants