No output in latest version

[withinboredom]

I don't have a simple reproduction case, but this causes nothing to be output when deep in the stack:

if(ob_get_level()> 0 && ob_get_length() > 0) {
	echo 'true';
}

If and only if those two conditions are on the same line (and if-statement) does it cause all output to cease functioning. Breaking them up into multiple if-statements is enough to allow processing to continue.

I'll bisect and try to hunt down the specific version this broke in. All I know is that it wasn't broken in

frankenphp version

FrankenPHP main Caddy v2.6.4 h1:2hwYqiRwk1tf3VruhMpLcYTg+11fCdr8S3jhNAdnPy8=

[withinboredom]

Ok, red herring. Here's the minimal reproduction case, no output is sent to the browser:

<?PHP

echo 'hello world';
flush();

// never executes
throw new LogicException('expected unhandled exception');

[withinboredom]

There's very clearly a test case for this in the repo and it works. After digging a little deeper when I removed:

RUN mv $PHP_INI_DIR/php.ini-production $PHP_INI_DIR/php.ini

from my Dockerfile, it magically started working as it did before. So now I'm trying to figure out which ini setting causes it to misbehave.

[withinboredom]

Looks like if there is output buffering, a flush() doesn't work.

[withinboredom]

When adding some logging to go_sapi_flush():

	flusher, ok := fc.responseWriter.(http.Flusher)
	if !ok {
	    fc.Logger.Info("response writer is not a flusher")
		return true
	}

I get the output:

{"level":"info","ts":1691355272.3962996,"logger":"http.handlers.php","msg":"response writer is not a flusher"}

[withinboredom]

But why did it work on an earlier version??

[withinboredom]

Potentially a bug with caddy 2.6 vs. 2.7?

[dunglas]

Maybe related to caddyserver/caddy#5509?

[dunglas]

It's not related to caddyserver/caddy#5509 after all.

The problem is that if the log directive is enabled, the caddyhttp.ResponseRecorder is used and it doesn't support streaming/http.Flusher. Removing the directive fixes the issue.
There is also another bug in FrankenPHP fixed in #179.

I don't know if this changed in Caddy 2.7, and if it's expected. Maybe @mholt has a hint about this?

[mholt]

To be sure, are you using Caddy 2.7.3?

[dunglas]

2.7.2 until #178 is merged, but I just tested and the problem also exists in 2.7.3.

[dunglas]

I cannot check right now but it's probably related to the ordering because with FrankenPHP using an ordered route is mandatory. @withinboredom could you try putting the log directive last?

[francislavoie]

The log directive isn't an HTTP handler directive, it sets up server-level config. Ordering is not the problem.

I think it does have to do with ResponseController stuff. Was the PHP handler updated to use ResponseController to call Flush()? It would need to, to unwrap the underlying flusher.

[mholt]

@dunglas I wish that were the case, but I think it's unlikely, since log is actually a directive that controls the HTTP server itself, it isn't ordered.

Edit: Oh, did we have a breaking change? Handlers can't just call Flush() without unwrapping first? (I think right now the typical code is to type-assert, i.e. rw.(Flusher) or similar)

[francislavoie]

Yeah, https://github.com/dunglas/frankenphp/blob/main/frankenphp.go#L568 needs to be updated to use ResponseController I think.

It's because of caddyserver/caddy#5654 which means our response writer isn't directly a flusher anymore and you need to use the response controller to flush now.

This should work:

http.NewResponseController(fc.responseWriter).Flush()

[mholt]

Sorry. I didn't realize this was a breaking change. 😖