Problem/Motivation

Under some circumstances, BigPipe throws PHP notices because it tries to explode on <drupal-big-pipe-scripts-bottom-marker> in \Drupal\big_pipe\Render\BigPipe::sendPreBody(), while that marker is missing.

This seems to happen when subrequests are created via DefaultExceptionHtmlSubscriber with javascript disabled.

Notices:

Notice: Undefined offset: 1 in Drupal\big_pipe\Render\BigPipe->sendPreBody() (line 143 of core/modules/big_pipe/src/Render/BigPipe.php).
Notice: Undefined offset: 2 in Drupal\big_pipe\Render\BigPipe->sendPreBody() (line 143 of d8/core/modules/big_pipe/src/Render/BigPipe.php).

Steps to reproduce:

On a fresh D8.1.x, create some content, disable javascript in your browser, visit /page-does-not-exist: this should trigger the above notices in the log.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mr.baileys created an issue. See original summary.

Wim Leers’s picture

Issue tags: +Needs tests

Reproduced. Thanks for the bug report! Working on test + fix.

Wim Leers’s picture

Title: PHP notices with javascript disabled on error pages » BigPipe causes PHP notices with JavaScript disabled on error pages
Assigned: Unassigned » Wim Leers
Status: Active » Needs review
Issue tags: -Needs tests
FileSize
1.45 KB
Wim Leers’s picture

FileSize
2.44 KB
1.01 KB

The root cause is this wrong assumption:

    // It does not make sense to have BigPipe responses for subrequests. BigPipe
    // is never useful internally in Drupal, only externally towards end users.
    $response = $event->getResponse();
    $is_eligible = $event->isMasterRequest() && $response instanceof HtmlResponse;
    $event->getRequest()->attributes->set(self::ATTRIBUTE_ELIGIBLE, $is_eligible);
    if (!$is_eligible) {
      return;
    }
Wim Leers’s picture

FileSize
3.54 KB
2.08 KB
Wim Leers’s picture

Assigned: Wim Leers » Unassigned
Issue tags: +Needs backport to D7

This should also be backported to https://www.drupal.org/project/big_pipe.

The last submitted patch, 3: 2679867-3.patch, failed testing.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

Version: 8.2.x-dev » 8.1.x-dev
Wim Leers’s picture

#2652766: PHP notice on page not found (when JS is disabled) was opened long before this, but the steps to reproduce were missing there. Berdir just reopened it with STR basically identical to this one. I closed that as a duplicate of this one.

Berdir’s picture

Thanks for the reference, somehow I only found the old issue, not this one.

Note that interestingly, I only got a single notice for 2, not 1. Although manual debugging showed that the explode only returned an array with a single key. I'll test this tomorrow.

I also don't have JS disabled, but maybe the issue title isn't correct about that?

mr.baileys’s picture

Status: Needs review » Reviewed & tested by the community

I also don't have JS disabled, but maybe the issue title isn't correct about that?

Technically it is the presence of the big_pipe_nojs-cookie that triggers the warnings. BigPipe::sendPreBody() returns early if there are no no-js placeholders, so the code throwing the warnings is only reached if there is at least one no-js placeholder. Wim concluded the same in #2652766-4: PHP notice on page not found (when JS is disabled).

I reviewed the patch and tested it with the BigPipe Demo blocks on both the system 40x messages as well as the custom 40x pages and can confirm that BigPipe works as expected for those subrequests, and no more notices are reported. The test added in #3 (https://www.drupal.org/pift-ci-job/199413) correctly triggers/tests the notices, and the patch in #5 makes the test pass.

Wim Leers’s picture

I think #12 is alluding to it, but I'll spell it out explicitly for #11: @Berdir, perhaps you don't have JS disabled, but you still have the BigPipe no-JS cookie?

Berdir’s picture

Confirmed, this fixes it for me as well.

Wim Leers’s picture

Perfect :)

Fabianx’s picture

RTBC + 1

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Less complexity and more tests - nice!

Committed 30cf748 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed c6e1b96 on 8.2.x
    Issue #2679867 by Wim Leers, mr.baileys: BigPipe causes PHP notices with...

  • alexpott committed 30cf748 on 8.1.x
    Issue #2679867 by Wim Leers, mr.baileys: BigPipe causes PHP notices with...
Wim Leers’s picture

Issue tags: -Needs backport to D7

Also committed & pushed to the contrib module for Drupal 8.0: http://drupalcode.org/project/big_pipe.git/commit/f0a2205.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

joseph.olstad’s picture

Version: 8.1.x-dev » 9.2.x-dev
Assigned: Unassigned » joseph.olstad
FileSize
1.01 KB

This isn't really fully fixed yet.

I created a module that takes data driven by linkchecker and does requests using Javascript that check for broken links on a site, it makes requests as fast as ajax can get the headers, doing this causes a zillion of these messages:

Notice: Undefined offset: 1 in Drupal\big_pipe\Render\BigPipe->sendPreBody() (line 333 of core/modules/big_pipe/src/Render/BigPipe.php).
Notice: Undefined offset: 2 in Drupal\big_pipe\Render\BigPipe->sendPreBody() (line 333 of core/modules/big_pipe/src/Render/BigPipe.php).

Fix is inspired by this idea:
https://stackoverflow.com/a/56971347

see patch:

joseph.olstad’s picture

created a new issue for this, triggered tests and they all pass.

#3194462: BigPipe when server under heavy load results in PHP notices

VVS’s picture

FileSize
1020 bytes

Fix patch from #22 to drupal 9.5

VVS’s picture

FileSize
941 bytes

upd