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

Catch notfound exception on deleting non existing file #122

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eXistenZNL
Copy link

Issue

Projects using FlySystem can use League\Flysystem\AdapterInterface::deleteDir() to remove a directory from a filesystem.
This should return a boolean to indicate whether this succeeded. This GCP implementation does however throw an exception.

This Pull Request fixes the issue.

An example where this happens is in Shopware 6, described here shopware/shopware#2108

Reproduction

  • Use this project as described in README.md.
  • Try to delete a directory from a GCP bucket that does not exist.
  • See that instead of false being returned, a Google\Cloud\Core\Exception\NotFoundException is being thrown
  • See that this can be an issue for downstream software relying on this repository.

The fix

The fix is quite easy actually: catch the exception thrown by the Google Cloud PHP API, so this implementation of the AdapterInterface matches the expected behaviour.

@eXistenZNL
Copy link
Author

@matthewgoslett can you have a look at this please? 👍

@eXistenZNL
Copy link
Author

@matthewgoslett poke ;)

@eXistenZNL
Copy link
Author

@matthewgoslett poke :)

1 similar comment
@eXistenZNL
Copy link
Author

@matthewgoslett poke :)

@matthewgoslett
Copy link
Contributor

matthewgoslett commented Jan 16, 2023 via email

@eXistenZNL
Copy link
Author

Hi @matthewgoslett thanks for letting me know, is there anyone else I could reach out to?

@eXistenZNL
Copy link
Author

@shad0wfir3 It seems you are the last person to have merged something, can I ask you to look at this PR? I'm not quite sure how to mention @superbalist/core from the CODEOWNERS file, but saw your name.

@eXistenZNL
Copy link
Author

@shad0wfir3 poke :)

1 similar comment
@eXistenZNL
Copy link
Author

@shad0wfir3 poke :)

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.

None yet

2 participants