Skip to content

Asynchronously handle streamed responses #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 27, 2016
Merged

Asynchronously handle streamed responses #40

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
786ae84
Asynchronously handle streamed responses
andig May 27, 2016
6f80ba0
Cleanup and add Content-Length
andig Jun 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 26 additions & 18 deletions Bridges/HttpKernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,20 @@ public function onRequest(ReactRequest $request, HttpResponse $response)

$syRequest = $this->mapRequest($request);

//start buffering the output, so cgi is not sending any http headers
//this is necessary because it would break session handling since
//headers_sent() returns true if any unbuffered output reaches cgi stdout.
ob_start();

try {
// start buffering the output, so cgi is not sending any http headers
// this is necessary because it would break session handling since
// headers_sent() returns true if any unbuffered output reaches cgi stdout.
ob_start();

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

$response->writeHead(500); // internal server error
$response->end();
Expand Down Expand Up @@ -170,15 +173,10 @@ protected function mapRequest(ReactRequest $reactRequest)
*/
protected function mapResponse(HttpResponse $reactResponse, SymfonyResponse $syResponse)
{
//end active session
// end active session
if (PHP_SESSION_ACTIVE === session_status()) {
session_write_close();
session_unset(); //reset $_SESSION
}

$content = $syResponse->getContent();
if ($syResponse instanceof SymfonyStreamedResponse) {
$syResponse->sendContent();
session_unset(); // reset $_SESSION
}

$nativeHeaders = [];
Expand All @@ -200,8 +198,8 @@ protected function mapResponse(HttpResponse $reactResponse, SymfonyResponse $syR
}
}

//after reading all headers we need to reset it, so next request
//operates on a clean header.
// after reading all headers we need to reset it, so next request
// operates on a clean header.
header_remove();

$headers = array_merge($nativeHeaders, $syResponse->headers->allPreserveCase());
Expand Down Expand Up @@ -240,12 +238,22 @@ protected function mapResponse(HttpResponse $reactResponse, SymfonyResponse $syR

$reactResponse->writeHead($syResponse->getStatusCode(), $headers);

$stdOut = '';
while ($buffer = @ob_get_clean()) {
$stdOut .= $buffer;
// asynchronously get content
ob_start(function($buffer) use ($reactResponse) {
$reactResponse->write($buffer);
Copy link

@duxet duxet May 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😶

Copy link
Contributor Author

@andig andig May 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcj

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 ;)

return '';
}, 4096);

if ($syResponse instanceof SymfonyStreamedResponse) {
$syResponse->sendContent();
}
else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no line break please

echo($syResponse->getContent());
}

$reactResponse->end($stdOut . $content);
// flush remaining content
@ob_end_flush();
$reactResponse->end();
}

/**
Expand Down