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

Profiling doesn't work with allow_url_fopen=Off #16

Closed
alies-dev opened this issue May 25, 2024 · 5 comments
Closed

Profiling doesn't work with allow_url_fopen=Off #16

alies-dev opened this issue May 25, 2024 · 5 comments

Comments

@alies-dev
Copy link
Contributor

Today I spent about 2h trying to fix xhprof integration and finally found an issue: it seems like HTTP client uses from underlying dependencies uses functionality that depends on allow_url_fopen PHP config value.

E.g. Guzzle throws an exception for such cases, making this obvious:

if (\ini_get('allow_url_fopen')) {
    $handler = $handler
        ? Proxy::wrapStreaming($handler, new StreamHandler())
        : new StreamHandler();
} elseif (!$handler) {
    throw new \RuntimeException('GuzzleHttp requires cURL, the allow_url_fopen ini setting, or a custom HTTP handler.');
}

I'm not sure what's better:

  • use another HTTP client
  • improve existing HTTP client dependency
  • document allow_url_fopen=On requirements

that's why I created this issue here. Please feel free to close it and solve or report it on a repository you think the best to solve this issue.

Thank you for the package!

@alies-dev
Copy link
Contributor Author

hey @maantje

Can you please take a look to this issue? I solved the same for another package InteractionDesignFoundation/laravel-geoip#35 and can do it for this package or it's dependencies. But I would like to know your opinion on which level we should solve the issue. thank you!

@maantje
Copy link
Owner

maantje commented Jun 20, 2024

Hi @alies-dev,

Apologies for the delay. I will review it this weekend.

Thank you for your patience.

@maantje
Copy link
Owner

maantje commented Jun 23, 2024

Hey @alies-dev,

I reviewed the issue, and I believe the best approach is to replace NativeHttpClient with CurlHttpClient. The spiral-packages/profiler package, which this project depends on, already includes ext-curl: *. If someone installs it with --ignore-platform-reqs, it should still fail due to this check in CurlHttpClient.

What do you think?

@alies-dev
Copy link
Contributor Author

@maantje
Agree, it solves the issue. Great stuff in #18 💪

maantje added a commit that referenced this issue Jun 25, 2024
* Switch out NativeHttpClient for CurlHttpClient #16 

---------

Co-authored-by: maantje <[email protected]>
@maantje
Copy link
Owner

maantje commented Jun 25, 2024

@alies-dev, Thank you for reporting and checking.

@maantje maantje closed this as completed Jun 25, 2024
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

No branches or pull requests

2 participants