maantje / xhprof-buggregator-laravel

Xhprof integration package in buggregator for Laravel
MIT License
25 stars 3 forks source link

Profiling doesn't work with `allow_url_fopen=Off` #16

Closed alies-dev closed 2 months ago

alies-dev commented 3 months ago

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:

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 commented 2 months ago

hey @maantje

Can you please take a look to this issue? I solved the same for another package https://github.com/InteractionDesignFoundation/laravel-geoip/pull/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 commented 2 months ago

Hi @alies-dev,

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

Thank you for your patience.

maantje commented 2 months ago

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 commented 2 months ago

@maantje Agree, it solves the issue. Great stuff in https://github.com/maantje/xhprof-buggregator-laravel/pull/18 💪

maantje commented 2 months ago

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