Storage Decorator can cause wrong logic to be performed due to overriding the "driver"

[grEvenX]

How do you use Sentry?

Self-hosted / on-premises

SDK version

3.8.0

Steps to reproduce

We're currently using the spatie/laravel-medialibrary package together with sentry-laravel.
After deploying the latest changes with the new approach to wrapping the disk configuration (introduced in #750) we experienced a problem uploading files to S3.

The changes in the given PR overrides the config("filesystems.disks.{$this->disk}.driver") for any given disk to become sentry. Any code that relies on checking which driver is being used in this way (through config) now fails to apply it's logic correctly.

Example logic in the Spatie media library where $diskDriverName is fetched from the config function above:

if (! in_array($diskDriverName, ['s3', 'gcs'], true)) {
  $this->filesystem->disk($diskName)->makeDirectory($directory);
}

PS: It still works in this case, the reason we caught this problem was that we hadn't enabled permission to create directories in S3, so it worked fine when not trying to create a directory but upload the fille "put()" directly.

Expected result

Enabling tracing on the Storage in a Laravel application should not affect logic

Actual result

Enabling tracing on the Storage in a Laravel application can potentially affect logic

[stayallive]

I'd be happy to hear if anyone thinks of a solution but I don't think there are any in this specific case. I don't know why they are checking the driver name since Flysystem should handle this for you (but again don't know the specifics of this check) without needing to know what underlying system is handling it. It might simply be an optimization since directories don't have any special meaning on most S3 like object storages. Although apparently you ran into an issue with permissions which is curious but I digress.

We went through great lengths documenting and minimizing the shortcomings of this approach but we can't code against these assumptions unfortunately. There is simply currently no way for us to profile the storage calls without proxying the driver like we are today because of how the drivers are built and setup.

So unless someone has a brainwave, that check is removed from the media library package or Laravel makes some changes you might have to resort to not enable storage profiling for those disks until we find a better less intrusive solution. Really sorry I don't have a "better" answer.

[grEvenX]

Yeah, I figured that it was done this way with "good reason". I'm pretty sure as you say that they could handle this differently in the library code, and I will probably raise an issue about it over there.
Just wanted to file this one so at least you are aware.
Haven't looked into the architecture enough yet to see if there's any other ways around this, but I'll shout out if I get time to look deeper into it.