Skip to content

Pagination discards all previously-fetched pages when any page fails, making 'empty' and 'failed partway' indistinguishable #355

Description

@rolandbanks

Summary

Every pagination helper in MerakiClientPager.cs accumulates results into a local List<T> and only returns it after the loop completes. If any page fails, the exception propagates and every page already fetched is silently discarded.

The practical effect is that a failure on page N of M is indistinguishable from a resource that is genuinely empty, because both reach the caller as the same exception with no partial data attached.

Location

Meraki.Api/MerakiClientPager.cs (all six pagination methods). Taking the two-cursor ApiResponse overload as the example (v1.70.37):

public static async Task<List<T>> GetAllAsync<T>(
    Func<string?, string?, CancellationToken, Task<ApiResponse<List<T>>>> pageFactoryAsync,
    CancellationToken cancellationToken)
{
    var allEntries = new List<T>();      // line 146 - a local
    var finished = false;
    ...
    while (!finished)
    {
        var pageResponse = await pageFactoryAsync(...).ConfigureAwait(false);

        // Refit traps exceptions into Error when using ApiResponse
        if (pageResponse.Error is not null)
        {
            throw pageResponse.Error;    // line 158 - abandons allEntries
        }

        allEntries.AddRange(pageResponse.Content ?? []);
        ...
    }

    return allEntries;                   // line 198 - never reached on error
}

Three of the six methods rethrow explicitly from ApiResponse.Error (lines 158, 233, 304); in the other three the factory's own exception propagates. In all six, allEntries is a local and is lost either way.

Why this matters

The Meraki API returns 404 for some genuinely empty collections. That means a consumer that wants to handle "this organization has no devices yet" has to catch ApiException with HttpStatusCode.NotFound and treat it as an empty result. That is a reasonable reading of the API.

But because the pager discards accumulated pages, that same catch also fires when page 7 of 7 fails on a large, fully populated organization. The two cases arrive at the caller as byte-identical inputs and require opposite correct responses:

Situation What the caller receives Correct response
Resource genuinely empty ApiException 404 Treat as empty
Page 7 of 7 failed ApiException 404 Retry, do not treat as empty

The information needed to tell them apart (that six pages of real data were already in hand) is destroyed inside the library, so no amount of care on the calling side can recover it. Any consumer that treats an empty collection as authoritative, for example to reconcile a local cache and remove items no longer present upstream, can be driven to discard a complete data set by a single transient page failure.

Suggested fixes

Any of these would restore the caller's ability to distinguish the two cases:

  1. Attach the partial results to the exception. Throw a dedicated exception type carrying the entries accumulated so far and the number of pages successfully fetched. Callers can then treat "404 on page 1 with 0 entries" differently from "404 on page 7 with 6 pages in hand". This is fully backwards compatible if the new type derives from the existing one.
  2. Return a result object exposing Entries, IsComplete and any error, instead of a bare List<T>. More invasive, but unambiguous.
  3. At minimum, document the behaviour on the GetAll*Async extension methods, so consumers know an exception means "discard everything" rather than "here is what I got".

Option 1 seems the smallest change with the most benefit.

Version

Observed in 1.70.37. The relevant code is unchanged on main as of this writing.

🤖 Filed with the assistance of Claude Code

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions