fix: plugin classes fail to load when the Composer classmap is stale - #2956
Conversation
Bundle Size Diff
|
|
Plugin build for 8148ac9 is ready 🛎️!
|
There was a problem hiding this comment.
Pull request overview
Prevents frontend fatal errors when an autoload-list class is unavailable.
Changes:
- Guards class instantiation with type and existence checks.
- Adds PHPUnit coverage for invalid and unavailable entries.
- Adds serialized E2E coverage for frontend and admin resilience.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
inc/class-main.php |
Skips unloadable autoload entries. |
tests/test-main-autoload.php |
Tests autoload resilience and bundled classes. |
packages/e2e-tests/mu-plugins/otter-e2e-bootstrap.php |
Adds the broken-autoloader scenario flag. |
src/blocks/test/e2e/blocks/autoloader-resilience.spec.js |
Verifies frontend and admin requests survive. |
src/blocks/test/e2e/playwright.config.js |
Serializes the stateful resilience spec. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@lucadobrescu let's research if we can fix the loading. Skipping the check is more like a compromise for a broken pipeline. |
E2E TestsPlaywright Test Status: See serial and parallel matrix jobs Performance ResultsserverResponse: {"q25":293.1,"q50":317.15,"q75":323.3,"cnt":10}, firstPaint: {"q25":1204.7,"q50":1296.65,"q75":1394.4,"cnt":10}, domContentLoaded: {"q25":2931.3,"q50":2934.05,"q75":2953.4,"cnt":10}, loaded: {"q25":2933,"q50":2935.8,"q75":2955.2,"cnt":10}, firstContentfulPaint: {"q25":3320.5,"q50":3333.85,"q75":3353.5,"cnt":10}, firstBlock: {"q25":11773.1,"q50":11844.75,"q75":11884.2,"cnt":10}, type: {"q25":24.84,"q50":26.89,"q75":30.14,"cnt":10}, typeWithoutInspector: {"q25":22.71,"q50":23.38,"q75":24.29,"cnt":10}, typeWithTopToolbar: {"q25":31.76,"q50":33.24,"q75":35.81,"cnt":10}, typeContainer: {"q25":14.41,"q50":15.3,"q75":17.3,"cnt":10}, focus: {"q25":113.74,"q50":121.83,"q75":130.38,"cnt":10}, inserterOpen: {"q25":38.71,"q50":39.41,"q75":40.84,"cnt":10}, inserterSearch: {"q25":14.41,"q50":14.89,"q75":15.83,"cnt":10}, inserterHover: {"q25":6.11,"q50":6.23,"q75":6.72,"cnt":20}, loadPatterns: {"q25":1533.31,"q50":1552.21,"q75":1564.14,"cnt":10}, listViewOpen: {"q25":191.12,"q50":195.72,"q75":209.15,"cnt":10} |
286494b to
fb75ec3
Compare
fb75ec3 to
b8a7fd9
Compare
|
I think the class_exists() check is enough here. A stale classmap is an environment issue (broken update, etc) and not something we can really fix on our side. Would be good to log something when a class is skipped though, otherwise it just looks like a feature stopped working for no reason. I'd remove the fallback autoloader part. It locks us into the file naming convention forever, adds a second way classes get loaded, and it could hide a genuinely broken release from us. Feels like a lot for something we've seen on one site so far, and it fixes itself when the user reinstalls the plugin anyway. If we see it happening more often we can revisit. What do you think @Soare-Robert-Daniel? |
There was a problem hiding this comment.
Is this class still needed?
|
@HardeepAsrani agreed, done. Dropped the fallback autoloader ( Nothing depends on the fallback since it was never released, so this just returns loading to Composer-classmap-only plus the guard. The e2e resilience spec still passes: it injects a class that has no file on disk, so the guard is what keeps the site up regardless. cc @Soare-Robert-Daniel |
The parser-collision guard silently served the stock animation stylesheet, so a conflicting Sabberworm release looked like the optimization had stopped working for no reason. Log the skip, mirroring the autoload-skip notice from #2956, so the cause is visible in the debug log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The parser-collision guard silently served the stock animation stylesheet, so a conflicting Sabberworm release looked like the optimization had stopped working for no reason. Log the skip, mirroring the autoload-skip notice from #2956, so the cause is visible in the debug log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bec2df1 to
8b62bc4
Compare
Main::autoload_classes() instantiated every entry of its class list without checking that the class is loadable. On a package with a stale Composer classmap the Atomic Wind entry resolved to nothing and `new $classname()` threw an uncaught Error on `init`, taking down every request (#2954). Skip entries that are not loadable so a packaging or third-party filter problem degrades to a missing feature instead of a site-wide fatal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tale Composer exposes `inc/` through a generated classmap in `vendor/`, so a map that does not match the files on disk — an interrupted plugin update, an OPcache entry compiled from the previous version — makes a class that is present unloadable. Register a fallback loader for `ThemeIsle\GutenbergBlocks\*` that resolves a class from its file name. It is appended to the SPL stack, so Composer still answers first and the fallback only runs when Composer has no answer. The class then loads and its feature keeps working, instead of being skipped by the guard in Main::autoload_classes(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Per review: a stale Composer classmap is an environment issue (broken or partial update) that fixes itself on reinstall, so keeping the class_exists() guard is enough. Remove inc/class-autoloader.php and its SPL registration — it locked us into the file-naming convention, added a second load path, and could hide a genuinely broken release. When the guard skips an unloadable entry it now logs it, so a lost feature is visible instead of silent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The parser-collision guard silently served the stock animation stylesheet, so a conflicting Sabberworm release looked like the optimization had stopped working for no reason. Log the skip, mirroring the autoload-skip notice from #2956, so the cause is visible in the debug log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8b62bc4 to
8148ac9
Compare
…S parsing (#2958) * fix: fatal when another plugin loads a different php-css-parser release Base_CSS::get_animation_css() parsed the animation stylesheet with the bundled Sabberworm parser even when another plugin had already loaded classes from a different release of it. Mixing the two fatals at class-link time ("Declaration of CSSList::addComments... must be compatible") and that error is not catchable, so a guard now verifies every sentinel class resolves to Otter's own vendor directory before anything parser-related loads — including the transient read, which stores parser objects. When the guard fails, the full stock otter-animation stylesheet is enqueued instead, matching the optimize-off delivery path. Adds an isolated-process PHPUnit regression test and a frontend e2e spec covering both the foreign-parser fallback and the bundled-parser optimized path. Fixes #2942 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: serve the stock animation stylesheet from the frontend loader on parser collision Copilot review: the get_animation_css() fallback enqueue was lost once the generated post CSS was cached — later requests never reach the parser path, and the cached CSS carries no animation rules. Move the fallback delivery into Blocks_Animation::frontend_load(), which runs on every request that renders an animated block, and cover a cached-CSS reload in the e2e spec. Also delete the spec's probe posts in afterAll. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: reject any preloaded foreign Sabberworm symbol, not only the sentinels Copilot review: the parser touches more classes than the three sentinels (OutputFormat, DeclarationBlock, KeyFrame, and the cached object graph), so a foreign copy of any of them slipped past the guard and still mixed releases. has_own_css_parser() now first rejects every already-declared class, interface, or trait under Sabberworm\CSS that does not resolve to the bundled vendor directory — before the sentinel checks can autoload anything — and only then resolves the sentinel entry points. The sandbox gains an outputformat scenario covering a foreign non-sentinel class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: match foreign Sabberworm declarations case-insensitively Copilot review (suppressed note): PHP class and namespace names are case-insensitive, so a foreign symbol declared with different casing is the same runtime class but slipped past the case-sensitive prefix scan. Use stripos so every colliding declaration is rejected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: trim animation-CSS collision comments to the load-bearing why Cut the verbose guard and preflight comments in Base_CSS and Blocks_Animation down to the constraint a reader cannot infer: the foreign-parser fatal is uncatchable, so the guard must run before any parser class loads. Also correct the has_own_css_parser() docblock to say it checks every loaded Sabberworm\CSS symbol, not only the ones the parser touches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: log when a foreign php-css-parser release disables animation CSS The parser-collision guard silently served the stock animation stylesheet, so a conflicting Sabberworm release looked like the optimization had stopped working for no reason. Log the skip, mirroring the autoload-skip notice from #2956, so the cause is visible in the debug log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Luca Dobrescu <luca.dobrescu@vertistudio.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Closes #2954.
Summary
Otter initializes a fixed list of classes on
initand callsnew $classname()on each one. A site on 3.2.0 hitClass "...Atomic_Wind_Blocks" not foundatinc/class-main.php:97, so every frontend request ended in a fatal error.The released packages are not the cause. Every wp.org zip since 3.1.6 — the first release with this class — ships
inc/plugins/class-atomic-wind-blocks.phpand its entry in both generated maps,vendor/composer/autoload_classmap.phpandvendor/composer/autoload_static.php. The class reaches runtime only through those generated maps, so it becomes unloadable when the map on the site stops matching the files on disk: an interrupted or partially applied plugin update, or an OPcache entry compiled from the previous version. That is site-side environment state, not something the plugin can repair, and it clears itself when the user reinstalls.So the fix stays small: don't let one unloadable entry take down the whole request.
Main::autoload_classes()— checksclass_exists()before instantiating, and skips non-string entries. The request loses one feature instead of the whole site. When an entry is skipped it is written to the error log, so a dropped feature is visible in the logs rather than silently gone.Note
An earlier revision of this PR also added a convention-based fallback autoloader. It was dropped in review: a stale classmap is an environment issue that fixes itself on reinstall, and the fallback locked us into the file-naming convention, added a second way classes load, and could hide a genuinely broken release. The
class_exists()guard plus a log line is enough. If we start seeing this on more than one site we can revisit.Class loading on init
flowchart LR A[init, priority 9] --> B[otter_blocks_autoloader<br/>filter: Pro + 3rd party] B --> C{class_exists?} C -- Yes --> F[Instantiate,<br/>call instance] C -- No --> E[Log + skip entry,<br/>continue list]:::added E --> G[Request renders] F --> G classDef added fill:#1a7f37,color:#fff,stroke:#116329,stroke-width:3pxTest instructions
Steps 1 and 2 create the state a stale map puts a site in.
Check out this branch and run
composer install. Openvendor/composer/autoload_classmap.phpand delete the line forAtomic_Wind_Blocks. Openvendor/composer/autoload_static.phpand delete the same line. Composer reads the static map first, so both files must lose the entry.Expect: neither file contains
Atomic_Wind_Blocks.Open
wp-config.php. SetWP_DEBUGtotrue, setWP_DEBUG_LOGtotrue, and setWP_DEBUG_DISPLAYtofalse. Deletewp-content/debug.logif the file exists.Expect: no log file is present.
Open the site home page in a browser.
Expect: the page renders with HTTP 200.
debug.logholds a[Otter Blocks] Skipped an autoload entry that could not be loaded: ...Atomic_Wind_Blocksline and noClass "..." not foundfatal. Ondevelopmentthe same request returns HTTP 500 and writes that fatal atinc/class-main.php:97.Run
composer dump-autoloadto rebuild the map. Reload the home page.Expect: the page renders with HTTP 200 and no skip line in
debug.log— the Atomic Wind module is loaded again.Checklist before the final review