0.34.x

[loks0n]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d29c187e-8381-420b-ab45-9fec0cd3a823

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 0.34.x

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR is a large refactor introducing adapter-based request/response classes for FPM, Swoole, and SwooleCoroutine, along with new telemetry instrumentation in Http::run(). Several issues raised in prior review threads remain unresolved (see inline comments from the previous round), and one new inconsistency was found: Swoole/Request::getReferer() ignores its $default parameter, returning a hardcoded empty string while the FPM adapter correctly passes $default through.

Confidence Score: 4/5

Safe to merge once prior-thread P1 items are addressed; the single new finding here is P2.

Multiple P1 items from prior review rounds remain open (activeRequests leak, \Exception vs \Throwable in hook catches, FPM onRequest/onStart ordering, Swoole getSize returning 0). The new finding (getReferer ignoring $default) is P2. Score of 4 reflects the unresolved P1s.

src/Http/Http.php (telemetry guard + Throwable catch), src/Http/Adapter/FPM/Server.php (start order), src/Http/Adapter/Swoole/Request.php (getReferer default + getSize override)

Important Files Changed

Filename	Overview
src/Http/Http.php	New telemetry layer wraps runInternal(); activeRequests counter lacks try/finally guard and onRequest catch uses \Exception instead of \Throwable — both flagged in prior review threads and still unresolved.
src/Http/Adapter/Swoole/Request.php	New Swoole request adapter; getReferer() silently ignores the $default parameter unlike the FPM equivalent; getSize() not overridden so body size reports 0 on Swoole (flagged in prior thread).
src/Http/Adapter/FPM/Server.php	onRequest() fires synchronously before onStart(), so start hooks run after the request is already processed — flagged in prior thread, still unresolved.
src/Http/Adapter/Swoole/Response.php	sendStatus() passes (string) cast to Swoole's int-typed status() method — flagged in prior thread; otherwise clean.
src/Http/Adapter/SwooleCoroutine/Server.php	$port passed without (int) cast to Swoole\Coroutine\Http\Server — flagged in prior thread; coroutine context cleanup with try/finally is correct.
src/Http/Request.php	getSize() reads php://input which is empty in Swoole; no override in Swoole adapters so telemetry body size will be zero — flagged in prior thread.
src/Http/Adapter/FPM/Request.php	New FPM request adapter; correctly implements all abstract methods including getReferer() passing $default through.
src/Http/Adapter/Swoole/Server.php	Missing try/finally around call_user_func for coroutine context cleanup (flagged in prior thread); request container scope otherwise correct.

_{Reviews (7): Last reviewed commit: "Lower Brotli default compression level t..." | Re-trigger Greptile}

[greptile-apps]

Unnecessary (string) cast in sendStatus

Swoole\Http\Response::status() has the signature status(int $http_status_code, string $reason = ''): bool. $statusCode is already typed as int by the parent abstract method — casting it to string before passing it in is at best a no-op (PHP coerces it back) and could cause a TypeError if Swoole ever enforces strict typing internally. The cast should be removed.

Suggested change

	$this->swoole->status((string) $statusCode);
	$this->swoole->status($statusCode);

[greptile-apps]

Missing try/finally cleanup for coroutine context

The SwooleCoroutine\Server variant wraps the callback invocation in a try/finally to unset(Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY]) after each request, ensuring stale per-request containers don't linger if an exception escapes. The process-mode Swoole\Server does not do this:

Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY] = $requestContainer;
\call_user_func($callback, new Request($request), new Response($response));
// No cleanup

Although Swoole destroys each request coroutine's context automatically when the coroutine exits, adding the matching try/finally here keeps the two adapters consistent and guards against any future change that reuses coroutines across requests:

Suggested change

	$this->server->on('request', function (SwooleRequest $request, SwooleResponse $response) use ($callback) {
	$requestContainer = new Container($this->container);
	$requestContainer->set('swooleRequest', fn () => $request);
	$requestContainer->set('swooleResponse', fn () => $response);
	Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY] = $requestContainer;
	\call_user_func($callback, new Request($request), new Response($response));
	Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY] = $requestContainer;
	try {
	\call_user_func($callback, new Request($request), new Response($response));
	} finally {
	unset(Coroutine::getContext()[self::REQUEST_CONTAINER_CONTEXT_KEY]);
	}

[greptile-apps]

getSize() reads php://input — incorrect for Swoole adapters

The base getSize() calls file_get_contents('php://input') to measure the request body size. This works under PHP-FPM, but in Swoole the request body is only accessible via $swoole->rawContent() — PHP's php://input stream is empty in that context.

Neither Swoole/Request.php nor SwooleCoroutine/Request.php override getSize(). Because Http::run() now calls $this->requestBodySize->record($request->getSize(), ...) for the new telemetry, all Swoole requests will report zero bytes for the body in the http.server.request.body.size metric.

Fix: override getSize() in Swoole/Request.php to use rawContent() for the body component:

public function getSize(): int
{
    $headers = $this->generateHeaders();
    $headerStrings = [];
    foreach ($headers as $key => $value) {
        $headerStrings[] = is_array($value)
            ? $key . ': ' . implode(', ', $value)
            : $key . ': ' . $value;
    }
    return mb_strlen(implode("\n", $headerStrings), '8bit')
         + mb_strlen($this->swoole->rawContent(), '8bit');
}

SwooleCoroutine/Request.php extends Swoole/Request.php, so it will inherit the fix.

[greptile-apps]

activeRequests counter leaks when runInternal throws

run() increments the active-requests counter before calling runInternal(), but the matching decrement sits after the call with no try/finally guard. runInternal can throw — for example, when an error handler itself raises an exception the code explicitly re-throws:

throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);

If that path is taken the add(-1) call on line 842 is skipped, and the http.server.active_requests gauge is permanently over-counted by one for every such request. Over time this makes the metric untrustworthy.

The fix is to wrap the post-call telemetry in a try/finally:

$start = microtime(true);
try {
    $result = $this->runInternal($request, $response);
} finally {
    $requestDuration = microtime(true) - $start;
    $attributes = [
        'url.scheme' => $request->getProtocol(),
        'http.request.method' => $request->getMethod(),
        'http.route' => $this->route?->getPath(),
        'http.response.status_code' => $response->getStatusCode(),
    ];
    $this->requestDuration->record($requestDuration, $attributes);
    $this->requestBodySize->record($request->getSize(), $attributes);
    $this->responseBodySize->record($response->getSize(), $attributes);
    $this->activeRequests->add(-1, [
        'http.request.method' => $request->getMethod(),
        'url.scheme' => $request->getProtocol(),
    ]);
}
return $result;

[greptile-apps]

Request hook errors not caught by error handler

The onRequest hook catch block uses \Exception, but PHP fatal errors (e.g., TypeError, ArithmeticError, Error) extend \Throwable, not \Exception. Any PHP internal error thrown inside a request hook will bypass this catch block entirely, skipping the error handler and propagating to the Swoole/FPM layer uncaught. The execute() method correctly uses \Throwable (line 745) — the same fix is needed here and at the start() hook catch (line 646).

Suggested change

	} catch (\Exception $e) {
	} catch (\Throwable $e) {

[greptile-apps]

onRequest fires before onStart, reversing initialization order

Http::start() calls $this->server->onRequest(…) first, then $this->server->onStart(…). For every other adapter that registers non-immediate callbacks this is fine, but here onRequest immediately invokes the callback, so the entire request is processed before the onStart callback (which sets up the 'server' resource and runs all Http::onStart() hooks) is ever called. Any start hook that sets up resources needed during request processing (database connections, config, etc.) will not have run yet, and injecting 'server' inside a route will fail with a "resource not found" error.

The fix is to store the start callback and fire it just before the request:

private $startCallback = null;

public function onRequest(callable $callback)
{
    $request = new Request();
    $response = new Response();

    $this->container->set('fpmRequest', fn () => $request);
    $this->container->set('fpmResponse', fn () => $response);

    if ($this->startCallback) {
        \call_user_func($this->startCallback, $this);
    }

    \call_user_func($callback, $request, $response);
}

public function onStart(callable $callback)
{
    $this->startCallback = $callback;
}