Skip to content

Update Http\Client\Exception to extend \Throwable #143

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

Closed
wants to merge 1 commit into from
Closed

Update Http\Client\Exception to extend \Throwable #143

wants to merge 1 commit into from

Conversation

ramsey
Copy link

@ramsey ramsey commented Oct 17, 2018

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #142
License MIT

What's in this PR?

Updates Http\Client\Exception to extend from \Throwable. Also includes dstuecken/php7ify to provide a \Throwable polyfill for versions of PHP earlier than 7.0.

Why?

Static analysis tools are complaining when creating HTTP clients that implement Http\Client\HttpClient

Example Usage

n/a

Checklist

  • Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix

Will update the CHANGELOG if this approach is acceptable to the maintainers.

To Do

n/a

Include dstuecken/php7ify to provide a \Throwable polyfill for versions
of PHP earlier than 7.0.

Resolves #142
@ramsey
Copy link
Author

ramsey commented Oct 17, 2018

The Travis CI build is failing only on HHVM, and I don't believe it's a result of this change. Here's the error:

Fatal error: Uncaught TypeError: Argument 1 passed to spec\Http\Client\Exception\HttpExceptionSpec::let() must implement interface Psr\Http\Message\RequestInterface, PhpSpec\Wrapper\Collaborator given in /home/travis/build/php-http/httplug/spec/Exception/HttpExceptionSpec.php:14
Stack trace:
#0 (): spec\Http\Client\Exception\HttpExceptionSpec->let()
#1 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Runner/Maintainer/LetAndLetgoMaintainer.php(52): ReflectionMethod->invokeArgs()
#2 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Runner/ExampleRunner.php(136): PhpSpec\Runner\Maintainer\LetAndLetgoMaintainer->prepare()
#3 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Runner/ExampleRunner.php(81): PhpSpec\Runner\ExampleRunner->executeExample()
#4 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Runner/SpecificationRunner.php(55): PhpSpec\Runner\ExampleRunner->run()
#5 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Runner/SuiteRunner.php(56): PhpSpec\Runner\SpecificationRunner->run()
#6 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Console/Command/RunCommand.php(172): PhpSpec\Runner\SuiteRunner->run()
#7 /home/travis/build/php-http/httplug/vendor/symfony/console/Command/Command.php(255): PhpSpec\Console\Command\RunCommand->execute()
#8 /home/travis/build/php-http/httplug/vendor/symfony/console/Application.php(964): Symfony\Component\Console\Command\Command->run()
#9 /home/travis/build/php-http/httplug/vendor/symfony/console/Application.php(248): Symfony\Component\Console\Application->doRunCommand()
#10 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/src/PhpSpec/Console/Application.php(104): Symfony\Component\Console\Application->doRun()
#11 /home/travis/build/php-http/httplug/vendor/symfony/console/Application.php(148): PhpSpec\Console\Application->doRun()
#12 /home/travis/build/php-http/httplug/vendor/phpspec/phpspec/bin/phpspec(26): Symfony\Component\Console\Application->run()
#13 {main}

@sagikazarmark
Copy link
Member

This looks like a duplicate of #144

@ramsey
Copy link
Author

ramsey commented Oct 17, 2018

Not exactly. Both this and #144 are alternate solutions to #142. 😄

@sagikazarmark
Copy link
Member

Ah, okay, I missed the polyfill. This one could actually be the (short-term) solution then, since we don't have any plans (yet?) to require PHP 7.x in HTTPlug 1.y.

@Nyholm Nyholm mentioned this pull request Oct 18, 2018
@dbu
Copy link
Contributor

dbu commented Oct 18, 2018

i would be fine with this change as a new minor 1.x version until 2.0 is released.

psr-18 is in vote phase - @Nyholm is this the last step and it becomes an official standard right after that, so we would release httplug 2.0 this month still?

re hhvm: yes, i think it has a tendency of deteriorating. we can ignore the failure in this PR as its not related (i restarted the latest master build and got the same: https://travis-ci.org/php-http/httplug/jobs/317959290). hhvm became completely irrelevant for all i know, i guess we should drop testing with it.

@dbu
Copy link
Contributor

dbu commented Oct 18, 2018

see #146 for hhvm

@Nyholm
Copy link
Member

Nyholm commented Oct 18, 2018

It's the last step

@dbu
Copy link
Contributor

dbu commented Oct 18, 2018

cool! is it worth it to still merge this, or do we not bother anymore?

there is no other significant change in master atm since the latest 1.1.0 release: v1.1.0...master

@Nyholm
Copy link
Member

Nyholm commented Oct 27, 2018

I don't think we should bother. =)

This is now fixed in master. Thank you @ramsey for this PR. Im going to close this.

@Nyholm Nyholm closed this Oct 27, 2018
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.

phpstan complains about Http\Client\Exception
4 participants