Blank screen served occasionally

[spotman]

Hi, @mmamedov !

After a couple of weeks of testing I may confirm the blank screen issue in my production environment. I have no enough data for fixing it so the better way is to create a test case for this issue and collect more debug info (coz production system is under high load and debugging is not reliable). As described in #9 my suggestion is to implement standalone test suite based on:

• page-cache
• monolog
• php internal web server for serving test page and HTTP client for fetching it.
  I think that we need to emulate concurrent requests to the page-cache and check response for blank screen. We may randomize content and headers so different conditions would be tested. We may check different PSR-16 implementations (biult-in and, for example, filesystem-based Symfony cache). Also there is a third-party package for testing any PSR-16 implementation so we may check is built-in one is processing correctly.

If you agree with this way, I may create this test suite in a separate branch and make a PR.

[mmamedov]

Sounds good Denis. Thanks! I will also try to figire this out.

…

On Fri 9. Feb 2018 at 12:00 AM, Denis Terekhov ***@***.***> wrote: Hi, @mmamedov <https://github.com/mmamedov> ! After a couple of weeks of testing I may confirm the blank screen issue in my production environment. I have no enough data for fixing it so the better way is to create a test case for this issue and collect more debug info (coz production system is under high load and debugging is not reliable). As described in #9 <#9> my suggestion is to implement standalone test suite based on: - page-cache - monolog - php internal web server for serving test page and HTTP client for fetching it. I think that we need to emulate concurrent requests to the page-cache and check response for blank screen. We may randomize content and headers so different conditions would be tested. We may check different PSR-16 implementations (biult-in and, for example, filesystem-based Symfony cache). Also there is a third-party package for testing any PSR-16 implementation so we may check is built-in one is processing correctly. If you agree with this way, I may create this test suite in a separate branch and make a PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADnp5fBmApLk9JwogqMjKB2cb1YPdo2pks5tS9ECgaJpZM4R_a6x> .

[spotman]

@mmamedov I've created a simplified test case with default configuration and concurrency but had not seen the blank page yet. May I know which logger/cache/strategy implementation you are using in your project? It will point us to the right direction.

[spotman]

@mmamedov I'd commited my current progress, please read commit message for more info. I will be away for couple of days and will continue to work on this issue right after return.
It looks like the race condition coz sometimes tests are passed and sometimes not. They are failing on the different assertions but the root problem is cache missing. I've changed default expiration time to 60 seconds and now issue-related tests are passing. So I think the problem is fixed coz there are a lot of tests covering headers, response statuses and length and all of them are passing right now. The only thing we need to update is the FileSystemCacheAdapter coz it is not fully PSR-16 compliant. I may do that on the next week.

[spotman]

My bad, I had not made git pull before, I'll merge your master into my issue10 branch and fix conflicts.

[spotman]

@mmamedov I've created a PR #11 so please review it and run checks in your dev environment (I'm using phpunit 6+ but running tests under phpunit 5+ seems to be OK).

[spotman]

@mmamedov had you seen my updates?

[mmamedov]

Hi @spotman! Thanks for the updates, great job!

There about 5k new lines :) I started reviewing, but will also need to check out this on my machine and test this.

One question - were you able to spot the issue with blank pages, and were you able to verify that these changes fix it?

First thing I noticed, is that there are quite a few syntax modification with spaces - I suggest we don't do this. In the long term this will result in many unnecessary files changes, as we add more variables to the code.

Example:

// Preferred way
$key = $this->getCurrentKey();
$this->currentItem = $this->getItemStorage()->get($key);

If you format as follow, you will need to change it when you introduce a longer variable:

// Not preferred, each time a new variable assignment is introduced you need to reformat whole block.
$key                          = $this->getCurrentKey();
$this->currentItem = $this->getItemStorage()->get($key);

Also a few other non trivial spacing changes, etc. This all makes you spend more time on this, and makes it difficult for someone else to review - no one like long code reviews.

Let's try to keep it in smaller increments, relevant to the issue we are trying to solve. Once we have a stable product that we like we could always have a quick overall code cleanup with cosmetic changes.
I think if every fix is this large, it will make it difficult to move faster.

Thanks!

[spotman]

@mmamedov sorry fo these unwanted changes with spacing, they occur upon built-in Reformat code dialog in PhpStorm which I use after every huge code change. I will configure the PhpStorm to not align statements by = symbol.

[mmamedov]

I also use a few plugins for coding standards, but usually when you only reformat your new code it works fine, instead of reformatting whole file you touched.

…

On Sat 3. Mar 2018 at 4:47 PM, Denis Terekhov ***@***.***> wrote: @mmamedov <https://github.com/mmamedov> sorry fo these unwanted changes with spacing, they occur upon built-in Reformat code dialog in PhpStorm which I use after every huge code change. I will configure the PhpStorm to not align statements by = symbol. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADnp5ea6cFo8KcMlVAgrnOv9ii1ENcr6ks5tarsJgaJpZM4R_a6x> .

[spotman]

Hmm, I need to spend some time for configuring PhpStorm, thanks for pointing me in this direction.

[spotman]

One question - were you able to spot the issue with blank pages, and were you able to verify that these changes fix it?

First of all, my primary goal was to create a test suite for producing the blank page issue. I've finished with a test suite which produces a lot of requests with semi-random configuration options and there was no blank page detected. The problem is that I had to slightly modify the original code for a CLI environment so some changes were made simply to make this test suite work. After that I've covered most configuration parameters and had not seen any blank issue. All combinations of the cache providers, loggers and other options does not generate empty page. So the answer is: no, I had not spotted this issue. But the test suite contains checks for a blank page or inconsistent response body. So if the blank page issue still exists then we need to update this test suite and randomize more options. Anyway, the library is passing all the tests and seems to work fine so there is no any risks to deploy this code to production. I will continue debugging the blank issue on my production servers but I also want to use the latest version without patching composer.json on multiple projects and kindly ask you to merge #11 (or help me to add more checks to the test suite by providing your page-cache config).