Skip to content

Negotiate error handler content type on every request - #3464

Open
iliaal wants to merge 1 commit into
slimphp:4.xfrom
iliaal:fix/error-handler-content-type-caching
Open

Negotiate error handler content type on every request#3464
iliaal wants to merge 1 commit into
slimphp:4.xfrom
iliaal:fix/error-handler-content-type-caching

Conversation

@iliaal

@iliaal iliaal commented Aug 23, 2026

Copy link
Copy Markdown

The ErrorHandler caches the content type negotiated from the first request's Accept header ($this->contentType is set only when currently null) and reuses it for the lifetime of the handler instance. Applications that share one ErrorHandler across requests, such as long-running worker runtimes (Swoole, RoadRunner, Laravel Octane) where the container-built handler persists, keep serving error responses in the first request's negotiated format regardless of later clients' Accept headers. A single JSON client pins every subsequent browser or XML client to JSON error pages until process restart.

This change recomputes the content type from each incoming request. The existing override semantics are preserved: once forceContentType() is called, the forced value wins for all subsequent requests.

The ErrorHandler cached the content type negotiated from the first
request's Accept header and reused it for the lifetime of the handler
instance. Applications sharing one handler across requests (long-running
worker runtimes such as Swoole, RoadRunner or Laravel Octane) kept
serving error responses in the first request's format regardless of
later clients' Accept headers.

Recompute the content type from each request unless forceContentType()
was called, which keeps its existing override semantics.
@odan

odan commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Thanks for the PR.

I think the bigger question here is whether Slim should support reusing the same application object graph across multiple requests at all.

Slim traditionally follows PHP shared-nothing request lifecycle. Long-running workers like RoadRunner, Swoole or Octane change that model and can keep the same objects alive between requests.

If we want to officially support that lifecycle, changing only $contentType is probably not enough. There are other stateful parts in Slim as well, for example in the internal routing graph contains mutable state such as Route::$arguments and RouteCollector. ErrorHandler itself stores the current request, exception, method and status code on the instance.

If we want to support persistent workers, I think we need to look at this more broadly and decide what kind of request reuse Slim should support. If this is not a lifecycle Slim intends to support, then I don't think we should start fixing individual cases for it here.

Also this patch changes the behavior of $handler->forceContentType(null); $isContentTypeForced remains true, so automatic negotiation would no longer happen afterwards.

@akrabat What do you think?

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 99.454% (-0.001%) from 99.455% — iliaal:fix/error-handler-content-type-caching into slimphp:4.x

@iliaal

iliaal commented Aug 23, 2026

Copy link
Copy Markdown
Author

The $contentType === null skip is the forceContentType() implementation from ab1abe0, not a worker-lifecycle cache. Null means "not forced." A first JSON Accept therefore sticks on a reused instance.

The other fields on ErrorHandler ($request, $exception, $method, $statusCode) are written from the current call at the top of __invoke. They don't leak into the next response. Route::$arguments is a real reuse issue; it's a different one.

forceContentType(null) currently re-enables negotiation on 4.x because null is the sentinel. I'll make forceContentType(null) clear $isContentTypeForced so that stays.

This PR doesn't try to make Slim a worker framework. It splits "forced" from "already negotiated" so the documented force API still works when the handler instance outlives one request.

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.

3 participants