[9.x] Storage::put() no longer throws an exception

[ankurk91]

• Laravel Version: 9.x
• PHP Version: Does not matter
• Database Driver & Version: Does not matter

Description:

Storage::put() and similar methods no longer throws and and silently return false, the actual error is never logged or visible to developer

Here is responsible code

framework/src/Illuminate/Filesystem/FilesystemAdapter.php

Lines 322 to 336 in d1c3d4f

	try {
	if ($contents instanceof StreamInterface) {
	$this->driver->writeStream($path, $contents->detach(), $options);
	return true;
	}
	is_resource($contents)
	? $this->driver->writeStream($path, $contents, $options)
	: $this->driver->write($path, $contents, $options);
	} catch (UnableToWriteFile $e) {
	return false;
	}
	return true;

PR #33612

Steps To Reproduce:

Update your .env with
FILESYSTEM_DISK=s3 then make a mistake in your AWS credentials or shutdown your minio server

Try to upload a file via Storage::put("test.text","Some text");

Notice that no exception will be thrown, nothing can be found in storage/logs either.
It is impossible to get underlying adaptor error, (in my case S3)

What can be done next

Throw the exception like before (v8.x) and let end user handle it. OR
Document this change in upgrade guide.

[driesvints]

Flysystem v2 & v3 now throw UnableToWriteFile so there's no way anymore to properly indicate why a write operation didn't succeed. For Laravel 9 we decided to consolidate all methods in the FilesystemAdapter to only work with boolean returns and catch all internal Flysystem exception throwing.

I've sent in a PR to the upgrade guide: laravel/docs#7755

We are however, maybe considering to implement an optional logger to log these exceptions instead.

[ankurk91]

Suppressing such exceptions is a bad idea.
Checking for boolean all over the project, whether uploading to S3 (cloud) failed or not, is no where developer friendly.
Laravel is known for developer happiness but no more. :(

Since this behaviour was not documented in upgrade guide, it should be easy to bring back old behaviour, or at-least throw a custom exception to halt the execution.

[driesvints]

Suppressing such exceptions is a bad idea.

I personally too, more prefer exception throwing. However, we decided to pursue boolean returns instead to consolidate the workings of the filesystem.

it should be easy to bring back old behaviour

We cannot do that as Laravel v9 is now a month old and doing this will introduce breaking changes for apps that already upgraded.

[mike-code]

Wow, that's so bad. You could at least give an option to choose between ExceptionThrowingFilesystemAdapter and UselessBooleanFilesystemAdapter. I came back to try Laravel after a few years because I almost forgot why I abandoned it in the first place. A quick reminder why I did that is always helpful.

ps: This (1) opened issue looked strange but now I see that you just arbitrarily make decisions and close issues immediatelly to end the discussion (though you don't lock the topic - kudos for that)

[taylorotwell]

Hey all! We have an idea for improving this for you - going to look into it.

[jjkavalam]

I am a newbie to Laravel, but experienced in other languages/frameworks. Quite impressed with Laravel's developer friendliness overall.

However, when trying to integrate with cloud storage, i did end up in a situation where there was no way to know what went wrong. I happened to see 'throw' => false by chance in config/filesystems.php and happily flipped it to true !

Could you please mention the existence of throw to the primary documentation page ?

Really appreciate all the hard work !
@driesvints @taylorotwell

[driesvints]

We can always review a PR if you can send one 👍

[jjkavalam]

@driesvints Sure, i have opened PR: laravel/docs#8961