[selul]

Requested by Marius · project thread

All Submissions:

• Have you followed the Optimole Contributing guideline?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes #1114.

Root cause. "Commands out of sync" is not caused by Optimole's queries. When a PHP fatal error (most often memory exhausted) hits while MySQL is still sending a query's rows, the mysqli connection is left with unread results, and every later query in that request fails with this error. WordPress then runs the shutdown callbacks on that broken connection, and Optimole's page processing is the first one to query (Page Profiler and attachment cache reads), so it logs one error per read on every affected request. The second report on the issue shows WordPress core's own _transient_doing_cron write failing the same way, which confirms the connection was already broken. Optimole has no raw mysqli, multi-query or unbuffered query code that could desynchronize it.

Reproduced outside WordPress with plain mysqli: a 300k-row SELECT under an 8 MB memory_limit dies with "Allowed memory size exhausted", and a SELECT 1 from a shutdown function then fails with Commands out of sync; you can't run this command now. With 64 MB both succeed.

Fix. New Optml_Manager::is_fatal_error( error_get_last() ), true only for E_ERROR, E_PARSE, E_CORE_ERROR and E_COMPILE_ERROR, which always end the request. When it is true:

• the output handler (handle_buffer_fallback(), the default path) returns the page unprocessed;
• the shutdown capture path (optml_capture_at_shutdown opt-in) echoes the captured HTML unprocessed instead of calling replace_content().

So a request that already died makes no profiler or cache reads, and stops filling the error log. Healthy requests, and requests with only warnings, notices or deprecations, are processed as before.

Not covered: the fatal error itself (the customer's memory limit or another plugin), and core's own shutdown queries, which still log the error on such requests. Support should point customers to the first fatal in their PHP log.

How to test the changes in this Pull Request:

1. On a test site, add a mu-plugin that renders an image and then exhausts memory late in the page, e.g. on wp_footer: add_action( 'wp_footer', function () { ini_set( 'memory_limit', '32M' ); $a = str_repeat( 'x', 64 * 1024 * 1024 ); } );
2. Load a frontend page with Optimole connected and Page Profiler active, with WP_DEBUG_LOG on.
3. Before this PR, the log shows Commands out of sync errors for _transient__oprof_... reads after the fatal (when the fatal hits mid-query). After it, only the original fatal is logged and the partial page is sent unprocessed.
4. Without the mu-plugin, pages are still rewritten to the CDN as before.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

Check	Result
PHPUnit full suite (WP trunk, PHP 8.3)	PASS, 440 tests
New tests/test-fatal-error-passthrough.php: the four fatal types are detected; warnings, notices, deprecations and no error are not; the output handler still rewrites the page after a warning	PASS
New Test_Buffer cases: with a simulated E_ERROR, the default in-handler mode and the shutdown-capture mode both output the page unprocessed; each fails when its guard is removed	PASS
Standalone mysqli repro of the root cause	Memory fatal mid-fetch, then shutdown query fails with "Commands out of sync"
phpstan on inc/manager.php, phpcs, PHP lint	PASS

Not proven: the fatal path through handle_buffer_fallback() inside PHPUnit (a real engine fatal can't be raised there without ending the run), and a live affected site. Codex review was skipped because the Codex CLI isn't available in this environment.

---

---

Generated by Claude Code

[pirate-bot]

Plugin build for c1acf05 is ready 🛎️!

• Download build
• Download dev

[selul]

@pirate-bot review

[pirate-bot]

🟡 Changes requested

The fatal pass-through works in both buffer modes, but no test fails without the change.

Validation details

• Files reviewed: 2/2 changed files.
• cd /repo; set -o pipefail; timeout 600 vendor/bin/phpunit --do-not-cache-result --filter 'Test_Fatal_Error_Passthrough::… printed OK (10 tests, 10 assertions) (run R3) The new test file passes at HEAD on PHP 8.3.35.
• cd /workspace/checks/RUN/base-tree; ln -sfn /workspace/checks/RUN/base-tree /var/www/html/wp-content/plugins/optimole-wp… printed ✔ Output handler processes page after a warning (run R5) This test also passes on the pr-base code.
• `mkdir -p /workspace/checks/RUN/integ; cat > /workspace/checks/RUN/integ/probe.php <<'EOF'

Untested areas

- The "Commands out of sync" error was not reproduced. The author's standalone mysqli repro was not rerun. - Issue #1114 does not show a fatal error in the customer's logs. If those requests had no fatal, profiler reads still log errors. - `sandbox-phpunit-ab.sh` could not load the base tree. The base run was done by hand.

🤖 Automated review · run code-review-agent_6ad8a09d24a657.12747870. --- **🤖 Review agent** — review posted ✅ on `ef765fad` · [changes requested · 1 finding](https://github.com//pull/1196#pullrequestreview-5599069096) · 15 min Run `code-review-agent_6ad8a09d24a657.12747870` · [trail](https://agents.vertistudio.com/admin/trails/github-pr/Codeinwp-optimole-wp-1196)

[pirate-bot]

🟡 Changes requested

The new tests now detect both fatal-error guards, but the new $error_reader seam needs a precise callable type.

Validation details

• Files reviewed: 2/2 files changed since the prior review, plus the earlier diff from run code-review-agent_6ad8a09d24a657.12747870 (not rerun).
• Both new Test_Buffer cases pass at HEAD on PHP 8.3.35: cd /repo && set -o pipefail; timeout 300 vendor/bin/phpunit --do-not-cache-result --filter 'fatal_error' --testdox 2>&1 … (run R3, exit 0).
• Removing the handle_buffer_fallback() guard fails the default-mode test at line 120 [R6].
• Removing the capture_and_process_buffer() guard fails the capture-mode test at line 139 [R8].
• tests/test-fatal-error-passthrough.php passes at HEAD [R12].

Untested areas

• The new tests cannot run on the base tree. The $error_reader property does not exist there [R11]. Guard-removal runs replace that check.
• A live site with a real memory fatal was not tested.

🤖 Automated review · run code-review-agent_6ad8a66be5a9a1.60894051.

---

🤖 Review agent — review posted ✅ on 08d08a52 · changes requested · 1 finding · 11 min

Run code-review-agent_6ad8a66be5a9a1.60894051 · trail

[pirate-bot]

🟢 Approved

The PR passes. The last change request is fixed: both error reader docblocks now state the exact callable type.

Validation details

• Files reviewed: 2/2 files changed since the prior review, plus the earlier diff from run code-review-agent_6ad8a66be5a9a1.60894051.
• Commit a29cee7 changes only two docblock lines. No runtime code changed.
• A source check matched the new type to error_get_last(), the test closure and the only caller.
• The fatal-guard test results come from run code-review-agent_6ad8a66be5a9a1.60894051 at 08d08a5, not rerun.

Untested areas

• PHPStan was not run on the new callable types. CI owns that run.
• A live site with a real memory fatal was not exercised.

🤖 Automated review · run code-review-agent_6ad8ab719598a1.64841850.

---

🤖 Review agent — review posted ✅ on a29cee79 · approved · 0 findings · 5 min

Run code-review-agent_6ad8ab719598a1.64841850 · trail

[selul]

E2E is failing at the Deploy to QA step with exit code 255, before any test runs. The step runs over SSH to the shared QA host, and exit 255 is that host refusing or dropping the connection. It is not related to this PR's changes. An earlier E2E run on the first commit hung on the same step from 10:12 to 12:28 UTC, as did the run on #1190. I've re-run the job once. If the QA host stays down, someone with access to it needs to check it.

---

Generated by Claude Code

[pirate-bot]

🟢 Approved

The PR passes. The only new commit merges the base branch and leaves the three PR files unchanged.

Validation details

• Files reviewed: 3/3 changed files.
• git diff a29cee7 HEAD on the three PR files is empty. The merge changes only inc/url_replacer.php and tests/test-replacer.php from the base branch.
• A source check confirmed each fix in the PR body: four fatal types only, handler pass-through and capture pass-through.
• Fatal-guard test results come from run code-review-agent_6ad8a66be5a9a1.60894051 at 08d08a5, not rerun.
• The approval of run code-review-agent_6ad8ab719598a1.64841850 at a29cee7 is reused.

Untested areas

• A live site with a real memory fatal was not exercised.
• Tests were not rerun on the new base. CI owns that run.

🤖 Automated review · run code-review-agent_6ad8c99950b5a8.76901314.

---

🤖 Review agent — review posted ✅ on c1acf05a · approved · 0 findings · 5 min

Run code-review-agent_6ad8c99950b5a8.76901314 · trail