Ruhrpottpatriot / GW2.NET

A user friendly wrapper around the official GW2 API
GNU General Public License v2.0
17 stars 17 forks source link

Regression in Paginator #35

Closed sliekens closed 8 years ago

sliekens commented 8 years ago

edit The RetryOnFault() addition to class Paginator introduced a threading issue in FindAllPagesAsync() that causes three errors:

Ruhrpottpatriot commented 8 years ago

Can you fix that? Or should I do it?

sliekens commented 8 years ago

edit: see updated first post original post I'm not sure if I understand what's happening.

It used to be possible to call FindAllPagesAsync(int, int) with the values of the X-Page headers of the first page, like so:

// The first page has properties for PageSize and PageCount
var firstPage = await colors.FindPageAsync(pageIndex: 0);

// Use the X-Page values of the first page to get the rest of the pages
var allPages = await colors.FindAllPagesAsync(firstPage.PageSize, firstPage.PageCount);

Now the same code throws a service exception on the last request (page index out of range).

# first page
GET https://api.guildwars2.com/v2/colors?page=0
-> X-Page-Size:50
-> X-Page-Total:10

# last page
GET https://api.guildwars2.com/v2/colors?page_size=50&page=10
-> page out of range. Use page values 0 - 9.

The last page index should be 9 instead of 10, but has it always been like that?

It's possible that the API has always behaved like that, and that this bug entered the code when I updated class Paginator with retry logic and task interleaving.

The code looks like it's doing the right thing.

for (var pageIndex = 0; pageIndex < pageCount; pageIndex++)

Maybe I'm going crazy...