Skip to content

fix: retry license reactivation soon when the store gives no verdict - #1739

Merged
jakejackson1 merged 2 commits into
hot-patch-6.17.1from
fix/license-reactivation-no-verdict-6.17
Sep 21, 2026
Merged

jakejackson1 merged 2 commits into
hot-patch-6.17.1from
fix/license-reactivation-no-verdict-6.17

Conversation

@jakejackson1

@jakejackson1 jakejackson1 commented Sep 21, 2026 •

Copy link
Copy Markdown
Member

When a site's URL changes, such as a site cloned to staging, the weekly license check reactivates the key for the new URL. If that reactivation request failed (a timeout, a server error or a rate limit), it was treated like the store refusing the key. The license screen's status was replaced with a generic error, and reactivation was put off for a week. The next weekly check could land just inside that backoff, so a single blip could delay reactivation by up to two weeks.

A failed request is now treated as no answer. Nothing is saved, so the license screen keeps the store's last status and message, and the license check runs again in three hours. If that retry also gets no answer, it falls back to the weekly backoff, so a store that keeps failing isn't retried every few hours. A real refusal (e.g. no activations left) still backs off for a week, as before.

Try it

npm run test:php -- --filter test_licensing_bulk_license_check

The new cases stub the store for the bulk check and the activation request. They cover a successful reactivation, a refusal, a network error and a 429 (each retried, then succeeding), two failures in a row, and the statuses and recorded URLs that are left alone. The no-answer cases fail on hot-patch-6.17.1.

Test plan

  • After a URL change, a failed reactivation request leaves the license status and message unchanged and schedules the bulk check about 3 hours out
  • A second failed request in a row backs off for a week, with no early retry
  • A refusal from the store backs off for a week, as before
  • Saving a license on the settings page still shows error / rate_limit when the store can't be reached
  • Test_Model_Settings, Test_Addon and Test_Settings pass
More info

Reading the response. Helper_Abstract_Addon::parse_license_response() returns the store's answer, or null when there isn't one: a WP_Error, a non-200 status, or a body with neither license nor error. update_license_status_from_response() now uses it as well, so the rule lives in one place. Its public behaviour is unchanged: a failure is still saved as error, and a 429 as rate_limit, which the settings page and the GPDF_LICENSE_KEY path rely on to show why activation failed.

sync_license_activation_url(). It gets the raw response from a new private request_license_activation(), which activate_license() also uses, and checks it before saving anything. With no answer it saves nothing and fires no gfpdf_addon_post_license_activation, so there's no error overwrite and no update-cache flush. It returns true, meaning "check again soon". The backoff is the existing gfpdf_license_url_change_{slug} transient, which now holds a state: retry (1 day) after the first no-answer, and blocked (1 week) after a refusal or a second no-answer. Only blocked stops an attempt, and a successful activation clears the state.

Retry scheduling. Model_Settings::licensing_bulk_license_check() schedules the retry when sync_license_activation_url() returns true. It and the bulk check's two existing "+3 hour" retries now share schedule_bulk_license_check_retry(), which keeps a retry that's already due sooner. The deprecated per-add-on schedule_license_check() ignores the return value. Nothing schedules its cron hook any more, and Controller_Upgrade_Routines clears leftover events.

Cleanup. activate_license_with_backoff() had one caller left, maybe_activate_hardcoded_license(), so it is inlined there with its 3-hour backoff unchanged.

Not in this PR. maybe_activate_hardcoded_license() still saves error over the stored status when the store can't be reached. It retries on the next admin request after 3 hours, so it recovers on its own. It's a separate change if wanted.

Manual test. On a dev site, the key was deactivated for the site's URL with the real store and a different URL recorded, so the real bulk check returned site_inactive. Success reactivated for real. A simulated timeout or 429 on the activation kept the real site_inactive status and message and scheduled the 3-hour retry, which then reactivated for real. Two failures in a row, and a simulated no_activations_left, each backed off for a week with no attempt on the next check.

Tests. Eleven cases in Test_Model_Settings, all run through the bulk check that drives reactivation in practice. Test_Model_Settings, Test_Addon, Test_Settings, Test_EDD_SL_Plugin_Updater and Test_Model_Install pass locally (167, 38 multisite skips), and PHPCS is clean. A live bulk check against the real store on a dev site left three licensed add-ons valid.

Re-activating a license after the site URL changed treated a failed request
like a refusal. A timeout, a 5xx or a 429 overwrote the store's last verdict
with a generic error and backed off for a week, and the next bulk check was a
week away too, so one blip could delay re-activation by up to two weeks.

sync_license_activation_url() now reads the raw response before saving
anything. With no verdict it saves nothing, so the license screen keeps the
store's last status and message and the update cache isn't flushed, and it
returns true so the bulk check runs again in three hours. A second request in a
row with no verdict falls back to the weekly backoff, so a store that keeps
failing isn't retried every few hours. A refusal still backs off for a week.
The backoff is one transient holding the attempt state, `retry` or `blocked`.

parse_license_response() is the one place a store response is read: it returns
the store's answer, or null when there is none, and
update_license_status_from_response() now uses it too, with 429 still shown as
rate_limit. activate_license() gets its request from request_license_activation()
so the re-activation can see the response first. The three-hour retries in
licensing_bulk_license_check() share one helper that keeps a retry already due
sooner. activate_license_with_backoff() had one caller left, the GPDF_LICENSE_KEY
path, so it is inlined there with its behaviour unchanged.

Tests cover re-activation through the bulk check, the statuses and recorded URLs
that are left alone, a refusal, a network error and a 429 each retried and then
succeeding, a second failure backing off, and an earlier retry being kept.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A successful re-activation after a request with no verdict left the `retry`
state in place for up to a day. If the same add-on then saw another URL change
and another failed request within that day, it went straight to the weekly
backoff instead of getting its three-hour retry. Found testing the retry path
against the store on a dev site.

A successful activation now deletes the backoff transient.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jakejackson1
jakejackson1 merged commit a7bb425 into hot-patch-6.17.1 Sep 21, 2026
13 checks passed
@jakejackson1
jakejackson1 deleted the fix/license-reactivation-no-verdict-6.17 branch September 21, 2026 06:55
jakejackson1 added a commit that referenced this pull request Sep 21, 2026
Ports the 6.17.1 release (6.17..6.17.1) to development:

- keep mPDF's cache folders through tmp cleanup, and keep font metrics for a week (#1731)
- stop concurrent PDFs failing with "Temporary files directory is not writable" (#1733)
- note a PDF left off a notification on the entry, linked to its settings and authored as the
  notification, with the Gravity PDF logo as its avatar (#1732, #1736)
- log generation errors with form/entry/PDF IDs instead of the whole object (#1732)
- licensing environment type, inactive license status and dead update packages (#1734)
- tag mPDF's log records with the form, entry and PDF they belong to (#1738)
- keep PDF URL paths and safe query args when redacting logs (#1737)
- retry license reactivation soon when the store gives no verdict (#1739)

Adapted to development: tests moved to tests/phpunit/integration on the shared TestCase,
Context_Logger uses the scoped GFPDF_Vendor\Psr\Log, the generation-error log context lives in
development's refactored Model_PDF::process_and_save_pdf(), and pdf_id defaults to '' for a
Helper_PDF built without a PDF ID. Adds the 6.17.1 changelog section. Left out: the version bump and the wp-env 11
CI change (development fixed the Debian 11 build its own way in #1724).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jakejackson1 added a commit that referenced this pull request Sep 21, 2026
Ports the 6.17.1 release (6.17..6.17.1) to development:

- keep mPDF's cache folders through tmp cleanup, and keep font metrics for a week (#1731)
- stop concurrent PDFs failing with "Temporary files directory is not writable" (#1733)
- note a PDF left off a notification on the entry, linked to its settings and authored as the
  notification, with the Gravity PDF logo as its avatar (#1732, #1736)
- log generation errors with form/entry/PDF IDs instead of the whole object (#1732)
- licensing environment type, inactive license status and dead update packages (#1734)
- tag mPDF's log records with the form, entry and PDF they belong to (#1738)
- keep PDF URL paths and safe query args when redacting logs (#1737)
- retry license reactivation soon when the store gives no verdict (#1739)

Adapted to development: tests moved to tests/phpunit/integration on the shared TestCase,
Context_Logger uses the scoped GFPDF_Vendor\Psr\Log, the generation-error log context lives in
development's refactored Model_PDF::process_and_save_pdf(), and pdf_id defaults to '' for a
Helper_PDF built without a PDF ID. Adds the 6.17.1 changelog section. Left out: the version bump.

Also takes 6.17.1's wp-env upgrade (^11.15.0, which repoints Debian 11 sources at archive.debian.org
itself) in place of #1724's tools/wp-env/patch-bullseye-apt.mjs, which the yarn wp-env scripts no longer run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant