`SqlitePersistentStorage` storage transition to `complete` until subsequent launch
markst opened this issue · 10 comments
Describe the bug
I'm using a StatefulWidget
to display a list of downloads. The widget structure is as follows:
Widget _buildDownloadsStream() {
return StreamBuilder<TaskUpdate>(
stream: _downloadManager.taskUpdates,
builder: (context, snapshot) {
return _buildDownloadsFuture();
},
);
}
Widget _buildDownloadsFuture() {
return FutureBuilder<List<DownloadItem>?>(
future: _downloadManager.downloads(),
builder: (context, downloads) {
if (downloads.connectionState == ConnectionState.waiting) {
return const Center(child: CircularProgressIndicator());
}
if (downloads.hasError) {
return Center(child: Text('Error: ${downloads.error}'));
}
final downloadItems = downloads.data ?? [];
if (downloadItems.isEmpty) {
return _buildEmptyDownloadsView();
}
return _buildDownloadsListView(downloadItems);
},
);
}
The downloads are fetched using the following method:
Future<List<DownloadItem>> downloads() async {
// Fetch new downloads
final tasks = await FileDownloader().database
.allRecordsWithStatus(TaskStatus.complete);
final downloads = tasks.map((task) {
return DownloadItem(
taskId: task.taskId,
title: path.basenameWithoutExtension(task.task.filename),
url: task.task.url,
filePath: task.task.filePath(),
complete: task.status == TaskStatus.complete,
);
}).toList();
return downloads;
}
I have a listener that updates metadata upon task completion:
_subscription = FileDownloader().updates.listen(
(update) {
// Forward update to all listeners
_updateController.add(update);
if (update is TaskStatusUpdate) {
// Store meta to disk:
if (update.status == TaskStatus.complete) {
_updateMeta(update.task)
.then((value) => debugPrint("Success storing meta " + value.toString()));
}
}
},
onError: (error) {
debugPrint("Error occurred: $error");
},
onDone: () {
debugPrint("Stream closed");
},
cancelOnError: true,
);
The Issue
Despite the listener correctly storing metadata and forwarding updates, the newly completed download is not included in the allRecordsWithStatus(TaskStatus.complete)
call when I attempt to refetch the downloads after a task completes. The new download only appears in the list after I relaunch the app.
Possible Cause
I suspect that calling FileDownloader().trackTasks()
might be related to triggering the markDownloadedComplete
operation, which finalizes the task. Interestingly, my listener receives another TaskStatusUpdate
event a second time, possibly indicating that the task state is being updated or finalized asynchronously after the initial completion.
Steps to Reproduce
- Start a download using the
FileDownloader
API. - Wait for the download to complete.
- Observe that the
allRecordsWithStatus(TaskStatus.complete)
call does not return the newly completed download. - Relaunch the app and observe that the download now appears in the list.
Expected Behavior
The newly completed download should be immediately available in the allRecordsWithStatus(TaskStatus.complete)
call after the task completes, without requiring an app relaunch.
Additional Context
- I am using
FileDownloader().trackTasks()
to ensure task tracking, but it seems the completion state isn't fully updated or reflected in the database until the app is restarted. - The listener appears to work as expected in terms of receiving task updates, but the issue might be related to when or how the task status is finalized in the database.
Even prior to using _downloadManager.taskUpdates
stream builder, this was the case.
If I use FileDownloader().database.allRecords()
it does provide entries.
However all the tasks are stuck in TaskStatus.running
until I relaunch.
As an alternative I've also used:
final tasks = await FileDownloader().database.allRecords();
final downloads = tasks //
.where((element) => element.status == TaskStatus.complete)
Further inspection does lead me to think it's only when using SqlitePersistentStorage
I've updated example to use a tab bar with a list.
Hopefully this example video demonstrates, as you can see the download list displays progress as 1.0 but the state is stuck as running
:
at 00:26 I relaunch the app and now the list displays items as complete
Screen_recording_20240904_104028.mp4
Demo without SqlitePersistentStorage
storage.
As you can see the state correctly transitions to complete
Screen_recording_20240904_104820.mp4
Thanks for the detailed analysis. I am not sure I follow your list update trigger entirely, but I wonder if the issue is related to how a task update is processed within the plugin:
- in
base_downloader.dart
function_emitStatusUpdate
(line 757) we call_updateTaskInDatabase
, importantly not waiting for the result of this call (which returns aFuture<void>
) - The actual database update is conditionally done in
_updateTaskInDatabase
by callingdatabase.updateRecord
(e.g .in line 879, 896 or 901) again without awaiting the result of the call (which returns a Future) - back in
_emitStatusUpdate
we call the registeredTaskStatusCallback
(line 771) or add the update to the updates stream (line 774)
The result of all this is that the call to the callback or listener is executed before the database update for the same record has completed. Therefore, if you use the arrival of an update on the updates
stream as a trigger to query the database, there is no guarantee that the record will have been updated, and you may show an old list. Unfortunately there is not currently a way to know at what time a record has in fact been updated.
Two caveats:
- I don't know if this is how you trigger the list updates, so not sure if this is the issue
- If it is, I cannot explain why it works for you with the default persistent storage and not with the sqlite one
The rationale for the unsynchronized approach is that database updates can take considerable time and slow down the processing of the actual event, which is why the database updates are not awaited. The lack of synchronization between the update and the database status is not ideal, but at the time I felt a reasonable cost for ensuring updates are delivered quickly.
Let me know what you think.
Might add that I did originally have a pull-to-refresh mechanism which triggered allRecordsWithStatus
future and results would not resolve with the completed states.
Here's is updated example I used for demo videos ( the SqlitePersistentStorage
is commented out )
main.dart.zip
Thanks - I was able to reproduce and debug. I suspect it is a concurrent write issue, where the last progress update is finalized after the final status update because the last progress update fetches an existing record (which takes time) before executing the write, so it ends up writing after the last status update. I guess my clever "let's not wait for the database update to complete" was not so clever after all...
I need to properly serialize the writes and need to think a bit about how to do that, so stay tuned.
Fixed in V8.5.5, Make sure to use pub get upgrade
to force upgrade of the underlying background_downloader package to this version. I have confirmed it fixes the issue, but let me know if you still run into something.
This is awesome! Thanks so much for explaining why and fixing.
How do I buy you a beer/coffee/donate?
Might add that I added a filter and throttle to the .taskUpdates
so that it only refreshes my list once we get completion events:
return StreamBuilder<TaskUpdate>(
stream: _downloadManager.taskUpdates
.where((update) => update is TaskStatusUpdate && update.status == TaskStatus.complete)
.throttleTime(const Duration(milliseconds: 500)),
builder: (context, snapshot) {
return _buildDownloadsFuture();
},
);
Interesting - I was not familiar with throttleTime
: is that from the RxDart package? And that sounds like a good solution (to what is arguably an imperfect decision on my part!).
As for support, I have a GitHub sponsor button that gives you some options. Thank you!