Revert to the previous context when a run(), runForMultiple() or central() callback throws (backport #1488) - #1489
Conversation
…l() callbacks throw
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Going to look at the 4.x PR since there are a few subtle changes that have been made in v4 (but not v3) that affect related functionality. |
Backport of #1488 to 3.x, extended to
Tenant::run(), which got itsfinallyon 4.x only (b9cc63f).On 3.x, none of the three helpers that switch context restores the previous one when its callback throws:
Tenant::run()stays initialized for that tenant.Tenancy::runForMultiple()stays initialized for the tenant whose callback threw.Tenancy::central()stays in the central context instead of re-initializing the previous tenant.The docblocks of
Tenant::run()andcentral()both say "Atomic, safely reverts to previous context".UpdateSyncedResourcewalks the tenants withrunForMultiple(), so a write refused in one tenant's copy leaves the rest of the request or job on that tenant.This PR wraps all three in
try/finally. When the callback returns normally nothing changes, including return values.This only changes what happens when a callback throws. If you'd rather not change that unconditionally on 3.x, I can put it behind a config option instead.
Tests, in
AutomaticModeTest:tenant_run_reverts_to_the_original_context_when_the_callback_throws, with#[TestWith]for a central and a tenant starting contextrun_for_multiple_reverts_to_the_original_context_when_the_callback_throws, samecentral_helper_reverts_back_to_tenant_context_when_the_callback_throwsAll five cases fail on 3.x and pass with the change, run locally on PHP 8.3 with Laravel 13 (the newest row of the CI matrix).