Race condition between page fetch and item list update causing item duplication

[Ahmed-Omar-Hommir]

Hi, I sincerely apologize for the delay in my response. @faisalansari0367 @EdsonBueno

Issue Description

While fetching the next page, if the user updates an item's state (for example, changing an item's 'favorite' status), the list state is updated through pagingController.itemList = updatedItems;. This also triggers another request for the same page.

REC-20230916174545.mp4

@EdsonBueno The issue also exists in the WonderWords app.

Note: I added a delay to my request to simulate scenarios, which does not mean that the problem rarely occurs. This has happened to me so many times, that even with the QA testing team, I had to copy and modify the package to implement a quick fix.

My Solution

The ongoing state is not enough. I added a new state called fetchingNextPage. This state will be used to indicate whether the client is in the process of fetching the next page. While the state is fetchingNextPage, any event that triggers a next page fetch will be ignored. This will prevent multiple requests for the same page number in the scenarios you described.

Before:

After:

Originally posted by @Ahmed-Omar-Hommir in #266 (comment)

[kyi87]

I have the same issue.

[mihalycsaba]

I noticed it once too, but like out of a hundred.

[rickypid]

I have the same issue.

I did add easy_debounce as suggested here

[mihalycsaba]

The app I make is new, since I used it more now, seems to me that it happens mostly when the app stays a little longer(5+ minutes) in the background. When I bring it back from the background it has the same data loaded as before, but when I refresh it freezes out for a few seconds and loads the first page two times.

[mihalycsaba]

I don't know how to work around this, I have only one listener on a page, I don't think a debouncer will solve this.

I always have duplication of the first page, after the app was in the background for a while. Even if I refresh multiple times it loads the first page two times, the data it loads is fresh, but for some reason first page load stays duplicated until I navigate to a different page and come back.

[mihalycsaba]

Sorry guys for the noise, my code was wrong, looks like I fixed it now. I added a new pagerequestlistener to the same controller two times, because I'm using a filter that changes. Instead that I'm just reinitializing the the controller before adding the listener.

[Ahmed-Omar-Hommir]

I have described my issue in this pull request #306 by adding a test case

@EdsonBueno

[LuuNgocLan]

Hi @Ahmed-Omar-Hommir

Remember to set nextPageKey to trigger the next page to be fetched. If not, the addPageRequestListener will execute _fetchData function when touching the end item of first page -> race condition call api

 _pagingController.addPageRequestListener((pageKey) {
      _fetchData(pageKey);
    });

when loading more new pages use appendPage and appendLastPage to indicator the last page (that will hide the end IndicatorLoading)

Example:

void _updateRemoteData(ConnectionsState state) {
    if (state.nextPageKey == null) {
      pagingController.appendLastPage(state.loadedmoreData ?? []);
    } else {
      pagingController.appendPage(state.loadedmoreData ?? [], state.nextPageKey);
    }
  }

If you want to update items in the current list, try this

 pagingController.value = PagingState(
            nextPageKey: THE_NEXT_PAGE_KEY, /// current item + next items want to load 
            itemList: itemsUpdated ?? [],
            error: null,
          );

if you want to refresh the list use pagingController.refresh() to clear list and pagingController.value = PagingState(...)

[r-i-c-o]

It is easy to catch this issue if you use key in paged list and change it together with calling pagingController.refresh(). Although it is not necessary to use key, it helps to see duplicating of items.

class ExampleScreen extends StatefulWidget {
  const ExampleScreen({super.key});

  @override
  State<ExampleScreen> createState() => _ExampleScreenState();
}

class _ExampleScreenState extends State<ExampleScreen> {
  int _resultPageVersion = 0;
  late final PagingController<int, ProductItem> _pagingController;

  @override
  void initState() {
    super.initState();

    _pagingController = PagingController(firstPageKey: 1);
    _pagingController.addPageRequestListener(_fetchPage);
  }

  void _fetchPage(int pageIndex) async {
    try {
      final newItems = await _exampleRepository.fetch(
        page: pageIndex,
        size: 10,
      );
      if (newItems.length < 10) {
        _pagingController.appendLastPage(newItems);
      } else {
        _pagingController.appendPage(newItems, pageIndex + 1);
      }
    } catch (error) {
      _pagingController.error = error;
    }
  }

  @override
  void dispose() {
    _pageLoadController.dispose();
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
        body: CustomScrollView(
          slivers: [
            PagedSliverList<int, ProductItem>(
              key: Key(_resultPageVersion.toString()),
              pagingController: _pagingController,
              builderDelegate: PagedChildBuilderDelegate(
                itemBuilder: (context, productItem, index) {
                  return ProductItemWidget(productItem);
                },
              ),
            ),
          ],
        ),
        floatingActionButton: FloatingActionButton(onPressed: () {
        setState(() {
          _pagingController.refresh();
          _resultPageVersion++;
        });
      }),
    );
  }
}

[clragon]

This is a very important issue that is unfortunately somewhat directly tied to how freely the controller can be updated.

I am considering how this could be resolved. The best idea I can come up with is that the controller itself contains a locking mechanism with which different parts of the app can essentially schedule transactions, much like database transactions.

However, there is a second problem that shows up with this.
We can make transactions that take place after each other, but a given transaction currently cannot "deduplicate" itself;
Because it cannot know that it has taken place. The state of the controller does not store which page keys have been "used", therefore, even when we request e.g. page 2 twice, these requests will not take place at the same time, they will still take place and duplicate the data.

Because of this, the controller would additionally need to know which page keys have been used.
It is unclear to me whether adding a set of used page keys to the controller is a good idea currently.
Generally, we want the user to have more control over the behaviour.

For my own application, I have created a complicated own controller class that translates to the controller class of this package,
found here: clragon/e1547/controller.dart
This controller is most likely too complex for most usecases. But it would be really convenient if similar functionality was possible through the expansion of the controllers in this package ,rather than through replacement.

I'll continue to think about this.

[Ahmed-Omar-Hommir]

Hello @clragon, I am very happy for your response

I solved my issue but am unsure if it caused any usage restrictions.

In my view, when you scroll down, the UI adds an event to fetch the next page. This is what causes the problem because it can add recurring events because it does not recognize that there is already a process waiting. The only condition it understands is that a new event must be added when you scroll down. This can happen even if you scroll up and down multiple times, adding multiple events with the same 'pageKey'.

To address this issue, I added a new state called "Fetch Next Page", or a similar name, to let the UI know that the operation is already in progress. This prevents multiple events from being added at once.

[Mohammadfazel03]

I have the same issue.