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

[8.x] Throw exception when unable to create LockableFile #36674

Merged
merged 4 commits into from
Mar 21, 2021

Conversation

JVMartin
Copy link
Contributor

@JVMartin JVMartin commented Mar 19, 2021

We run into this cryptic error in our production Laravel servers:
image

This is because LockableFile incorrectly presumes that the result of fopen (which is stored in $this->handle) is always a file pointer resource, when in fact it can be the boolean value false when the file failed to create properly - relevant docs. When the file fails to create, $this->handle silently gets set to false here:

protected function createResource($path, $mode)
{
$this->handle = @fopen($path, $mode);
}

...such that subsequent references to $this->handle will fail with the above cryptic message, when in fact the real error happened much earlier when the file was supposed to be created.

If, instead, we throw an exception when fopen fails, the developer will see a helpful error showing the actual path of the file which failed to create, so that we can action upon that, whether it be updating the group permissions or umask of the storage folder or whatever is needed. This will be a much better developer experience then waiting for a subsequent usage of $this->handle to fail cryptically.

I'm not sure exactly what exception should be thrown here, please let me know how this can be improved.

@JVMartin JVMartin changed the title [8.x] Throw exception when unable to create file [8.x] Throw exception when unable to create LockableFile Mar 19, 2021
@driesvints
Copy link
Member

Maybe just remove the error suppression instead?

@taylorotwell taylorotwell merged commit 03a297f into laravel:8.x Mar 21, 2021
@JVMartin JVMartin deleted the filesystem-exception branch March 21, 2021 20:50
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

3 participants