Bug: `StoryBulkApi::all()`: Incomplete retry handling

[coajaxial]

This issue is generated using AI, so it may contain small errors (especially in the refactor example at the end), but the core bug is described correctly, I have proof-read it :)

Expected Behavior

• StoryBulkApi::all() should robustly handle HTTP 429 (Too Many Requests) responses by retrying the same page request up to MAX_RETRIES.
• The method should initialize $totalPages from the first successful response and then paginate through all pages, retrying transient 429 errors without breaking pagination.
• If retries are exhausted, throw a StoryblokApiException with a clear message.

Current Behavior

• If the first request (line 67) returns 429, isOk() is false and the code enters handleErrorResponse() (line 162), which only sleeps for a second and returns. Because there is no retry of the same request inside the current iteration, $totalPages remains null. The subsequent do...while condition at line 89 (while ($page->page() <= $totalPages)) then fails due to $totalPages being null, terminating pagination prematurely without yielding any stories.
• On later pages, a 429 triggers the same behavior: the code sleeps and increments $retryCount (lines 79–81) but does not retry the same request within a bounded retry loop. It relies on the outer do...while to loop again. This results in:
  • No explicit retry for the current request within the same iteration.
  • Potentially hitting MAX_RETRIES in handleErrorResponse() and throwing without ever actually retrying the just-failed request in a controlled retry loop.

Steps to Reproduce

1. Use StoryBulkApi::all() with any valid parameters that would normally return stories.
2. Simulate or provoke a 429 from the API on the very first call to page() (line 67). For example, throttle the endpoint or mock StoryApi::page() to return a StoryblokResponseInterface with status 429 for the first call.
3. Observe:
   • handleErrorResponse() (lines 162–170) sleeps and returns.
   • $totalPages is still null because no successful response occurred (lines 148–155 only run when isOk() is true).
   • The do...while condition at line 89 ends the loop immediately. No items are yielded.
4. For a second scenario, allow the first page to succeed (200) and then make the second page return 429:
   • The loop sleeps and increments $retryCount (lines 79–81) but does not retry the same request within a bounded retry loop. Depending on timing and repeated 429s, it can exhaust retries and throw without successfully retrying the failed page fetch.

Suggested Fix

Introduce a proper retry loop around each page fetch (including the very first one). Retry the same request up to MAX_RETRIES. Only proceed when a request is isOk(). If the page never succeeds within the retry budget, throw a clear exception.

Example refactor sketch:

public function all(?StoriesParams $params = null, ?QueryFilters $filters = null, int $itemsPerPage = self::DEFAULT_ITEMS_PER_PAGE): \Generator
{
    $totalPages = null;
    $page = new PaginationParams(self::DEFAULT_PAGE, $itemsPerPage);

    do {
        $retryCount = 0;

        while (true) {
            try {
                $response = $this->api->page(params: $params, queryFilters: $filters, page: $page);

                if ($response->isOk()) {
                    $totalPages = $this->handleSuccessfulResponse($response, $totalPages, $itemsPerPage);
                    // yield current page items
                    yield from $this->getStoriesFromResponse($response);
                    $page->incrementPage();
                    break; // exit retry loop for this page
                }

                // Handle non-OK responses (incl. 429) with bounded retries
                if ($response->getResponseStatusCode() === self::RATE_LIMIT_STATUS_CODE) {
                    if ($retryCount >= self::MAX_RETRIES) {
                        throw new StoryblokApiException('Rate limit exceeded maximum retries for page ' . $page->page());
                    }
                    $this->handleRateLimit();
                    ++$retryCount;
                    continue; // retry same page
                }

                $this->logger->error('API error', [
                    'status' => $response->getResponseStatusCode(),
                    'message' => $response->getErrorMessage(),
                    'page' => $page->page(),
                ]);
                throw new StoryblokApiException($response->getErrorMessage());

            } catch (\Exception $e) {
                // If exceptions carry HTTP 429 code (similar to createStories), treat them the same
                if ($e->getCode() === self::RATE_LIMIT_STATUS_CODE && $retryCount < self::MAX_RETRIES) {
                    $this->handleRateLimit();
                    ++$retryCount;
                    continue; // retry same page
                }

                $this->logger->error('Error fetching stories', [
                    'error' => $e->getMessage(),
                    'page' => $page->page(),
                ]);
                throw new StoryblokApiException('Failed to fetch stories: ' . $e->getMessage(), 0, $e);
            }
        }
    } while ($totalPages !== null && $page->page() <= $totalPages);
}

Notes:

• This mirrors the explicit retry handling already used in createStories() (lines 101–138), making behavior consistent across read/write operations.
• The loop condition guards with $totalPages !== null for safety, though the retry loop ensures the first page must succeed (or throw) before evaluating the condition.

[roberto-butti]

Thank you @coajaxial , for the detailed explanation and for taking the time to test and identify this potential issue. I really appreciate the thorough information you’ve provided; it’s very helpful. I’ll take a closer look based on your findings.

Thanks again!

Roberto