Asynchronously handle streamed responses #40
Conversation
| $stdOut .= $buffer; | ||
| // asynchronously get content | ||
| ob_start(function($buffer) use ($reactResponse) { | ||
| $reactResponse->write($buffer); |
There was a problem hiding this comment.
I have no idea why, but this line makes my whole response body empty, with a not streamed response 😟
edit: it's working with if ($buffer), but output buffer is not added to final response 😶
There was a problem hiding this comment.
I have no idea why, but this line makes my whole response body empty, with a not streamed response
but output buffer is not added to final response
Could you elaborate what that means? Which OS and which PHP version? I've had success testing this on Debian Jessie, php 5.6.
Does it mean that streamed response is working for you but not the regular response?
There was a problem hiding this comment.
Since all stuff here is asycnhronouse we can not do async stuff here. That will break since reactResponse->end() is called immediately some lines below. Also I don't get the point of using async method here because between this line and $reactResponse->end is nothing that can do something async.
There was a problem hiding this comment.
Since all stuff here is asycnhronouse we can not do async stuff here. That will break since reactResponse->end() is called immediately some lines below. Also I don't get the point of using async method here because between this line and $reactResponse->end is nothing that can do something async.
I'm not sure what you mean. We get multiple ob callbacks for a single sendContent so that is async in a manner. $reactResponse->end is only called after sendContent finishes (and ob_end_flush is called)- so all buffers have been cleared.
What could I do to resolve the issue you see? I have tried this ;)
|
@duxet PR is updated. Non-streamed response is working now, too. It entirely escapes me why the content was not written before. It was definitely captured. @marcj for sake of clarity I've split the streamed/ non-streamed path (though they could be combined). Difference to previous implementation is that STDOUT output for normal response is silently swallowed now- if you need it echoed please give me a hint. One thing I've noticed during testing is that we're always sending chunked responses even in case of normal response where content length is known if upstream response doesn't contain |
| if ($this->bootstrap instanceof HooksInterface) { | ||
| $this->bootstrap->preHandle($this->application); | ||
| } | ||
|
|
||
| $syResponse = $this->application->handle($syRequest); | ||
|
|
||
| // should not receive output from application->handle() | ||
| ob_end_clean(); | ||
| } catch (\Exception $exception) { |
There was a problem hiding this comment.
We need a ob_end_clean call also in catch block I think. I guess its better to just let ob_start out of try and add
ob_end_clean after the whole try-catch.
Btw, tabs detected :P
There was a problem hiding this comment.
We need a ob_end_clean call also in catch block I think. I guess its better to just let ob_start out of try and add
ob_end_clean after the whole try-catch.
Btw, tabs detected :P
@marcj Will fix this. Could you also comment on the content-length?
|
PR updated and content-length for none-streamed response added. @duxet please give it a try |
|
ok, i will try when i will be back home today |
|
ping @duxet would be great if you could confirm that the non-streamed response is working, too. According to my tests the issue has been fixed. |
|
ping? |
| } | ||
| else { |
|
Thanks, good work! |
This PR will send the
StreamingResponsein 4k blocks instead of as chunk. This should improve performance and reduce memory usage.Fixes #36