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

[Filesystem] improve error handling in lock() #22910

Merged
merged 1 commit into from May 28, 2017

Conversation

Projects
None yet
3 participants
@xabbuh
Member

xabbuh commented May 25, 2017

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #22882
License MIT
Doc PR
Show outdated Hide outdated src/Symfony/Component/Filesystem/Tests/LockHandlerTest.php
}
if (!$e instanceof IOException) {
$this->fail(sprintf('Expected IOException to be thrown, got %s instead.', get_class($e)));

This comment has been minimized.

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

is this required if we remove the two last "catch"?

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

is this required if we remove the two last "catch"?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

hum ok, for cleanup

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

hum ok, for cleanup

Show outdated Hide outdated src/Symfony/Component/Filesystem/Tests/LockHandlerTest.php
$this->fail(sprintf('Expected IOException to be thrown, got %s instead.', get_class($e)));
}
if (null !== $wrongMessage) {

This comment has been minimized.

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

assertNull?

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

assertNull?

Show outdated Hide outdated src/Symfony/Component/Filesystem/LockHandler.php
@@ -82,8 +86,12 @@ public function lock($blocking = false)
restore_error_handler();
if (!$this->handle) {
$error = error_get_last();
throw new IOException($error['message'], 0, null, $this->file);
if (null === $error) {

This comment has been minimized.

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

is this possible at all? to me it shouldn't

@nicolas-grekas

nicolas-grekas May 25, 2017

Member

is this possible at all? to me it shouldn't

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas May 28, 2017

Member

Thank you @xabbuh.

Member

nicolas-grekas commented May 28, 2017

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit 25381a2 into symfony:2.7 May 28, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request May 28, 2017

bug #22910 [Filesystem] improve error handling in lock() (xabbuh)
This PR was merged into the 2.7 branch.

Discussion
----------

[Filesystem] improve error handling in lock()

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22882
| License       | MIT
| Doc PR        |

Commits
-------

25381a2 [Filesystem] improve error handling in lock()

@xabbuh xabbuh deleted the xabbuh:issue-22882 branch May 28, 2017

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