Preserve root exception when bulk scheduling fails in BulkManagement/MassSchedule

[ddevallan]

Description

BulkManagement::scheduleBulk() wraps its DB and publishing work in a try/catch that logs the exception message and returns false — discarding the exception object entirely. MassSchedule::publishMass() checks the return value, gets false, and throws:

throw new LocalizedException(__('Something went wrong while processing the request.'));

No $previous is attached. The root cause is gone. An operator or integrator receiving this error has nothing to work with — not the exception type, not the message, not the stack trace of the actual failure.

Reproduction

Force a failure inside the try block in scheduleBulk() and call the async bulk REST endpoint:

curl -X POST /rest/async/bulk/V1/products \
  -H "Authorization: Bearer $TOKEN" \
  -d '[{"product":{"sku":"test","status":1}}]'

Before fix:

{"message": "Something went wrong while processing the request."}

No trace back to BulkManagement. Root cause (e.g. "RabbitMQ connection refused") is invisible.

Fix — two files, one PR

BulkManagement: Store the last caught exception in $this->lastException. Expose it via getLastException().

} catch (Exception $exception) {
    $connection->rollBack();
    $this->logger->critical($exception->getMessage());
    $this->lastException = $exception;   // ← preserved
    return false;
}

public function getLastException(): ?Exception
{
    return $this->lastException;
}

MassSchedule: Retrieve the stored exception and pass it as $previous to LocalizedException.

if (!$this->bulkManagement->scheduleBulk(...)) {
    $previous = method_exists($this->bulkManagement, 'getLastException')
        ? $this->bulkManagement->getLastException()
        : null;
    throw new LocalizedException(
        __('Something went wrong while processing the request.'),
        $previous   // ← root cause now attached
    );
}

The method_exists() guard ensures no BC break if a third-party implements BulkManagementInterface without getLastException().

Impact of the fix

The message to API consumers remains "Something went wrong while processing the request." — no change there. The benefit is in the exception chain:

• Error monitoring tools (Sentry, Bugsnag, New Relic) that walk getPrevious() will now surface the root cause
• Custom exception handlers and middleware can call $e->getPrevious()->getMessage() to get the actual error
• Log decorators that capture the full chain will include the root cause

Test results

Bulk Management (Magento\AsynchronousOperations\Test\Unit\Model\BulkManagement)
 ✔ Schedule bulk
 ✔ Schedule bulk with exception
 ✔ Get last exception returns exception caught by schedule bulk
 ✔ Schedule bulk with exception during publishing
 ✔ Retry bulk
 ✔ Retry bulk with exception
 ✔ Delete bulk

OK (7 tests, 91 assertions)

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated — N/A
• All automated tests passed successfully (all builds are green)

[marcelmtz]

This is really cool, thank you! @thebraziliandeveloper qq. Does it make sense to clear the last exception in the scheduleBulk? 🤔
$this->lastException = null

[rhoerr]

@ddevallan Any thoughts on Marcel's question above? Thanks

[rhoerr]

Fable assessment:

Design concerns

• Stateful error channel on a shared singleton. BulkManagement is a DI singleton; $lastException is per-instance mutable state. It's never reset on a successful call, so a later false-return path (or any future caller of getLastException()) can pick up a stale exception from an unrelated earlier failure in the same process — a real risk in queue consumers and long-running processes, where the misattributed trace would be actively misleading. At minimum, reset $this->lastException = null at the top of scheduleBulk().
• method_exists() against an interface-typed dependency. MassSchedule types $bulkManagement against BulkManagementInterface, so the guard is admitting this is off-contract. It works, but it's duck typing in core code and the pattern tends to get cherry-picked as precedent.
• Cleaner alternative worth considering: since Mage-OS can't add to the Adobe API interface without a BC break, the more conventional fix is for scheduleBulk() to throw a dedicated wrapped exception (e.g. BulkException) that MassSchedule catches and chains — or simpler and fully BC-safe: have MassSchedule do nothing and instead have BulkManagement log with context ($this->logger->critical($exception) passes the full trace to the logger, which is one line and fixes the observability gap for operators without any API surface change). If the goal is specifically Sentry/getPrevious() chains, the current approach does deliver that, but the one-line logging fix covers most of the stated pain.

Correctness

• The two MassSchedule throw sites are handled correctly, including capturing $previous before deleteBulk() runs (which could itself overwrite state) — good ordering.
• Note deleteBulk() and retryBulk() don't set $lastException, so getLastException() is specifically "last scheduleBulk failure"; the docblock says so, which is fine.

Tests

• The new BulkManagement test is solid (asserts null-before, same-instance-after).
• Missing: a MassSchedule test asserting the thrown LocalizedException's getPrevious() is the root cause — that's the actual behavior this PR exists to deliver, and it's untested. The PR body's test list covers only BulkManagementTest.
• Minor style: the aligned = assignments in the new test don't match surrounding Magento style and may trip phpcs.

Not merging yet

[marcelmtz]

Not merging yet per last comments