[gonzaloebiz]

The library calls back into the host's helper in four places. Two of them assume a method the host may not have, and have done since 3.0.42.

What breaks, and for whom

saveNotification() and getGmtDate() belong to the module. Composer cannot hold that pairing together:

• the module requires a minimum library version;
• a library cannot require a minimum module version — the dependency only exists in one direction;
• and the module's constraint is a floor with no ceiling (>=3.0.52 today, >=3.0.38 when this landed).

So composer update on an old installation pairs whatever module is on disk with the newest library published, and an app/code install — extension by clone, library by separate require — pairs the two under no constraint at all.

saveNotification() arrived in the module on 2024-12-03, in 103.4.65. The calls to it landed here the day after, first tagged 3.0.42. Every module below 103.4.65 has been a fatal on both call sites ever since:

Mailchimp.php:364        on every call that answered      2xx
Error.php:92             on every call that failed        4xx

Between them that is most of what the extension does.

The error path is the worse of the two. It is reached because Mailchimp answered 4xx, so an installation that cannot listen loses the message explaining the failure and the request that was carrying it, at the moment it needs both most. A host with no way to be notified is not an error to raise; it is a host to leave alone.

Shape

Guarded per method, not per helper — the discipline the telemetry already follows in readContactAllowed() and readModuleVersion(). getGmtDate() has been in the module since 2018 and is guarded for the same reason rather than because it is likely to be absent: nothing here can assume the shape of the host it was handed.

Nothing changes for a current module. The payload, the call and the order are the same; only the assumption is gone.

Test

The error lane is covered directly through the public getFriendlyMessage(), across four host shapes — both methods, neither, getGmtDate() only (the real shape of a module below 103.4.65), and no helper at all — plus Mailchimp_HttpError, which inherits the guard.

The lane inside Mailchimp::call() needs a curl round trip to Mailchimp to reach, and a test that needed the network to prove a guard would be skipped exactly when it mattered. So the rule is asserted over the source instead: every $this->helper->x() in src must be guarded on the method it names, in every file, including ones added later. Without this change that assertion names all four call sites.

Verified against a real installation as well, with a helper carrying getGmtDate() and no saveNotification(), on a call that answers 200:

before   Error -> Call to undefined method ...::saveNotification()
after    the call returned, account_id present

Not in scope

The library cannot tell a host that a notification was dropped, and does not try. A module too old to store one is also too old to be told about it.

[gonzaloebiz]

Both taken, in caae96a.

The hardcoded list. You are right that it was the one blind spot the test exists to cover, and the description was the part that was true — the code lagged it. It now walks src recursively with scandir() and plain recursion rather than the SPL iterators, which were build-optional before 5.3 and this runner declares the same php >=5.2.0 the library does.

Checked rather than assumed: dropping an unguarded $this->helper->tellTheHostSomething() into a new class under a new src/Mailchimp/Probe/ directory now fails the run and names the file, with the count going 64 → 65.

  65 files scanned
  [FAIL] none found: src/Mailchimp/Probe/Newcomer.php -> tellTheHostSomething()

Per file, not per call site. Also right, and worth having in writing since the argument for the test is that it pins the rule. Reading a guard back to the call it protects needs a parser, which this runner cannot have, so the comment now states the narrower thing it actually proves — no file calls a helper method it has never checked for — and names the case where that is weaker than per-call-site: one method, two calls, one guard, which is what the current sources look like.

Against develop's src, the run still fails 7 of 17 and exits 1, so the assertion has not gone soft in the process.

[dipola]

Verified caae96a independently rather than reading it — ran the guard test three ways on PHP 8:

1. the branch as it stands          64 files scanned · 17/17 · exit 0
2. an unguarded $this->helper->x()  65 files scanned · FAIL, exit 1
   in a NEW src/Mailchimp/Probe/      names src/Mailchimp/Probe/Newcomer.php -> tellTheHostSomething()
3. the same test against the         7 failed
   pre-fix sources                    names all four call sites in both files

Run 2 is the one that matters: the file did not exist, its directory did not exist, and the scan found it. The 64 → 65 count reproduces exactly. Run 3 confirms the assertion did not go soft while being widened — it still fails loudly on sources without the guards.

The scandir() + recursion choice is the right call and not just a stylistic one: RecursiveDirectoryIterator would have quietly tied a test that exists to protect php >=5.2.0 installations to a build where SPL happened to be compiled in.

And narrowing the comment to "no file calls a helper method it has never checked for", with the weak case named, is better than the broader claim it replaced. A test that overstates what it proves is worse than one that proves less, because the next person trusts it.

LGTM.

[dipola]

Verified caae96a rather than read it — the scan now catches an unguarded call in a file and a directory that did not exist before, and it still fails against the pre-fix sources, so it was widened without going soft.

The scandir() choice keeps the php >=5.2.0 promise the runner makes, which RecursiveDirectoryIterator would have quietly broken.

Good fix, and the error path was the right one to prioritise.