781flyingdutchman / background_downloader

Flutter plugin for file downloads and uploads
Other
158 stars 73 forks source link

downloadStatusCallback not being called on android #6

Closed The-Redhat closed 9 months ago

The-Redhat commented 1 year ago

Hey

thanks for the awesome plugin!

Everything works flawlessly on ios. On android on the other hand the downloadStatusCallback or downloadProgressCallback is not being called after at all. The native logs appear in the logcat indicating a successful download.

During my small debugging, everything seemed to work on the native side, but in the flutter the _backgroundChannel MethodCallHandler never gets called.

Thanks in advance

781flyingdutchman commented 1 year ago

Hi, sorry to hear that.

I haven't come across this issue before, so a few questions to see if we can figure this out: 1) Can you share the Android, Dart and Flutter versions you are running? 2) Do you know if the example app works? 3) If so, can you confirm that you've set the .progressupdates property of the download task to provide updates? 4) Can you share the code where you initialize the downloader and enqueue the task?

Thanks!

On Thu, Jan 19, 2023, 12:25 AM Gregor Weber @.***> wrote:

Hey

thanks for the awesome plugin!

Everything works flawlessly on ios. On android on the other hand the downloadStatusCallback or downloadProgressCallback is not being called after at all. The native logs appear in the logcat indicating a successful download.

During my small debugging, everything seemed to work on the native side, but in the flutter the _backgroundChannel MethodCallHandler never gets called.

Thanks in advance

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF5VCH6XSH3LWRQUGDBLY4DWTD3BJANCNFSM6AAAAAAUAA63RM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Maqcel commented 1 year ago

@781flyingdutchman Hi I came across your package mostly because I can't find a way to download files to documents folder via flutter_downloader package on Android. Which is a big deal for my offline functionality. I've tried your solution and it's working just like @The-Redhat said on ios only. I'm experiencing same issues that callbacks doesn't inform flutter back about the progress or statuses of downloads. download method is waiting for eternity for the future while logs about it's success pass by :(. But I'll answer some of the questions u asked: 1) Flutter 3.3.9 • channel stable • https://github.com/flutter/flutter.git Framework • revision b8f7f1f986 (8 weeks ago) • 2022-11-23 06:43:51 +0900 Engine • revision 8f2221fbef Tools • Dart 2.18.5 • DevTools 2.15.0 Android: Pixel_3a_API_33_arm64-v8a emulator 2) Example page worked fine when I've tried it as initial page 3) I've tried both callbacks status/progress via DownloadTaskProgressUpdates.statusChangeAndProgressUpdates and the default one 4) Not fully but I'll try to give you perspective:

Future<void> downloadStep(Some params) async  {

 FileDownloader.initialize(
      downloadProgressCallback: (task, progress) => log(progress.toString()),
      downloadStatusCallback: (task, progress) => log(progress.toString()),
    );
    if (!await _downloadAuthorAndCoursePhotos(
      courseContent,
    )) {
    /// sth more error handling
    }
}
Future<bool> _downloadAuthorAndCoursePhotos(
  CourseContentResponse course
) async {
  try {
    const String coverPhotoFileName = 'cover.jpg';
    const String authorPhotoFileName = 'author.jpg';
    if (course.author.profileImage?.url != null) {
      await FileDownloader.download(BackgroundDownloadTask(
        url: course.author.profileImage!.url,
        filename: authorPhotoFileName,
        baseDirectory: BaseDirectory.applicationDocuments,
        directory: 'courses/${course.courseId}',
        progressUpdates:
            DownloadTaskProgressUpdates.statusChangeAndProgressUpdates,
      ));
    }
    await FileDownloader.download(BackgroundDownloadTask(
      url: course.photo.url,
      filename: coverPhotoFileName,
      baseDirectory: BaseDirectory.applicationDocuments,
      directory: 'courses/${course.courseId}',
    ));
    return true;
  } catch (_) {
    return false;
  }

The problem in this might be with dependency injection. I'm using injectable, bloc and auto_route packages. With this my structure looks sth like that: Cubits are injected to pages via wrappedRoute and

BlocProvider<SomeCubit>(
        create: (BuildContext context) => getIt<SomeCubit>(),
)
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:injectable/injectable.dart';

import 'some_state.dart';

@injectable
class SomeCubit extends Cubit<SomeState> {
  final SomeUseCase _someUseCase;

  SomeCubit(this._someUseCase)
      : super(const SomeState.loading());
}

SomeUseCase calls method from Service that starts downloading in the case from above author and cover photos. And I think that this can somehow interfere with communication but I'm left without the clue there

781flyingdutchman commented 1 year ago

@Maqcel thanks for sharing the issue and your code. The issue you're encountering is that if you use any of the 'convenience' download methods .download or .downloadBatch then no callback will be called, because those methods simply wait for the download to finish, and return the DownloadStatus. So in your example, if you want to do the two downloads (for cover photo and author photo) in parallel and use your callbacks, then you should use Filedownloader.enqueue instead of Filedownloader.download and confirm that the result of the call to enqueue is true.

If you want to use the convenience methods then you don't need any callbacks (they won't be called!), but should store the result of the call, e.g.:

final result = await FileDownloader.download(BackgroundDownloadTask(
      url: course.photo.url,
      filename: coverPhotoFileName,
      baseDirectory: BaseDirectory.applicationDocuments,
      directory: 'courses/${course.courseId}',
    ));
if (result == DownloadTaskStatus.complete) {
    print('success');
} else {
  print('Failure $result');
}

Note though that because the convenience functions are sequential (i.e. they wait for completion), using .download will execute one after the other. You can also conveniently download both cover and author photos in parallel by putting the two tasks in a list and calling Filedownloader.downloadBatch. All files in a batch will download concurrently, and the result of the call is an object that you can check for success, e.g.:

final result = await FileDownloader.downloadBatch(
    [BackgroundDownloadTask(
      url: course.photo.url,
      filename: coverPhotoFileName,
      baseDirectory: BaseDirectory.applicationDocuments,
      directory: 'courses/${course.courseId}',
    ),
    BackgroundDownloadTask(
      url: course.author.profileImage!.url,
      filename: authorPhotoFileName,
      baseDirectory: BaseDirectory.applicationDocuments,
      directory: 'courses/${course.courseId}',
    )]);
if (result.numFailed == 0) {
  print('Success');
} else {
  print('Failed ${result.failed.toList()});
}

I think this will solve your problem, but let me know if not.

Note that the reason your callbacks are not called when using convenience methods is that those convenience methods use their own callbacks to accomplish their job. They also use a special .group parameter, so when using a convenience method the fields group and progressUpdates of the BackgroundDownloadTask are ignored/overwritten.

781flyingdutchman commented 1 year ago

Hi, I realize that what I wrote may not actually address the issues both of you raised w.r.t. the callback not getting called on Android. I wonder if adding @pragma('vm:entry-point') above your callback function addresses your issue: it tells the compiler not to 'tree-shake' that callback. I'm torn, because in my own apps that use the downloader I don't have to do that, and also because that would only be relevant in release mode (and you appear to be using an emulator in debug mode, when tree shaking doesn't happen) BUT the issue you describe does sound like a tree-shaking issue (where the compiler removes the callback because it can find no evidence of it ever getting called). Can you try, and let me know if that solves your problem? If not, are there any logs or further code you can share, because I don't understand what could cause the issue you're experiencing and I really want to solve it! Thanks!

The-Redhat commented 1 year ago

Hey I tried adding @pragma('vm:entry-point') but it did not change anything.

Here are the logs for a download

V/BackgroundDownloaderPlugin(28207): Starting task with id 3958078286
I/DownloadWorker(28207):  Starting download for taskId 3958078286
I/DownloadWorker(28207): Successfully downloaded taskId 3958078286 to /data/user/0/de.knowunity.app/app_flutter/BQfgrjyrIlqzJWbjnRMN_PREVIEW_SMALL.webp
I/WM-WorkerWrapper(28207): Worker result SUCCESS for Work [ id=99cc9c0a-64ce-4e62-beb5-70017e4c8ef5, tags={ taskId=3958078286, com.bbflight.background_downloader.DownloadWorker, BackgroundDownloaderPlugin, group=default } ]

Code for downloading

 final task = BackgroundDownloadTask(
      url: fileUrl,
      filename: basename(fileUrl),
      metaData: identifier,
    );
    FileDownloader.enqueue(task);

I tried the example and it works for me.

Here the steps I took so far to debug the problem:

  1. I added custom logging to the native processStatusUpdate method inside the DownloadWorker class.
    
        fun processStatusUpdate(
            backgroundDownloadTask: BackgroundDownloadTask,
            status: DownloadTaskStatus
        ) { 
            Log.i(
                TAG,
                "processStatusUpdate called"
             )
            if (backgroundDownloadTask.providesStatusUpdates()) {
                Log.i(
                        TAG,
                        "backgroundDownloadTask.providesStatusUpdates() is true"
                )
                Handler(Looper.getMainLooper()).post {
                    try {
                        val gson = Gson()
                        val arg = listOf<Any>(
                            gson.toJson(backgroundDownloadTask.toJsonMap()),
                            status.ordinal
                        )
                        Log.i(
                                TAG,
                                "send statusUpdate to flutter"
                        )
                        BackgroundDownloaderPlugin.backgroundChannel?.invokeMethod(
                            "statusUpdate",
                            arg
                        )
                    } catch (e: Exception) {
                        Log.w(TAG, "Exception trying to post status update: ${e.message}")
                    }
                }
            }
            // if task is in final state, process a final progressUpdate and remove from
            // persistent storage
            if (status != DownloadTaskStatus.running) {
                when (status) {
                    DownloadTaskStatus.complete -> processProgressUpdate(
                        backgroundDownloadTask,
                        1.0
                    )
                    DownloadTaskStatus.failed -> processProgressUpdate(backgroundDownloadTask, -1.0)
                    DownloadTaskStatus.canceled -> processProgressUpdate(
                        backgroundDownloadTask,
                        -2.0
                    )
                    DownloadTaskStatus.notFound -> processProgressUpdate(
                        backgroundDownloadTask,
                        -3.0
                    )
                    else -> {}
                }
                BackgroundDownloaderPlugin.prefsLock.write {
                    val jsonString =
                        BackgroundDownloaderPlugin.prefs.getString(
                            BackgroundDownloaderPlugin.keyTasksMap,
                            "{}"
                        )
                    val backgroundDownloadTaskMap =
                        BackgroundDownloaderPlugin.gson.fromJson<Map<String, Any>>(
                            jsonString,
                            BackgroundDownloaderPlugin.mapType
                        ).toMutableMap()
                    backgroundDownloadTaskMap.remove(backgroundDownloadTask.taskId)
                    val editor = BackgroundDownloaderPlugin.prefs.edit()
                    editor.putString(
                        BackgroundDownloaderPlugin.keyTasksMap,
                        BackgroundDownloaderPlugin.gson.toJson(backgroundDownloadTaskMap)
                    )
                    editor.apply()
                }
            }
        }
All three logs appear in the console on android.

2. I added a setMethodCallHandler to the apps main method.

void main() { final widgetsBinding = WidgetsFlutterBinding.ensureInitialized(); FlutterNativeSplash.preserve(widgetsBinding: widgetsBinding);

await CrashReportingService.instance.init(); await Firebase.initializeApp();

MobileAds.instance.initialize();

FileDownloader.initialize( downloadStatusCallback: folderDownloadStatusCallback, );

MethodChannel('com.bbflight.background_downloader.background') .setMethodCallHandler((call) async { print('received download background call: $call'); });

// ... some other code }


On ios the print statement is visible in the console, on android not.
The-Redhat commented 1 year ago

I added a check for the result of the backgroudChannel invocation

   BackgroundDownloaderPlugin.backgroundChannel?.invokeMethod("statusUpdate", arg, object : MethodChannel.Result {
                            override fun success(result: Any?) {
                                Log.i(
                                        TAG,
                                        "method channel success"
                                )
                            }

                            override fun error(code: String, msg: String?, details: Any?) {
                                Log.i(
                                        TAG,
                                        "method channel error"
                                )
                            }

                            override fun notImplemented() {
                                Log.i(
                                        TAG,
                                        "method channel not implemented"
                                )
                            }
                        })

It prints a method channel not implemented in the console. Do you know why this is happening?

781flyingdutchman commented 1 year ago

Thanks so much for your help debugging this. Can you add one more log statement just before the .invokeMethod call in Kotlin, to confirm that baxkgroundChannel is not null? If it is null, can you add logs to the onAttachedToEngine and onDetachedFromEngine calls to understand why it's not getting set? My current thinking is that that may be the issue, though I have to admit that I don't understand why this would happen, especially since it's working for you in the example app!

On Sat, Jan 21, 2023, 8:45 AM Gregor Weber @.***> wrote:

Hey I tried adding @pragma('vm:entry-point') but it did not change anything.

Here are the logs for a download

V/BackgroundDownloaderPlugin(28207): Starting task with id 3958078286 I/DownloadWorker(28207): Starting download for taskId 3958078286 I/DownloadWorker(28207): Successfully downloaded taskId 3958078286 to /data/user/0/de.knowunity.app/app_flutter/BQfgrjyrIlqzJWbjnRMN_PREVIEW_SMALL.webp I/WM-WorkerWrapper(28207) http://de.knowunity.app/app_flutter/BQfgrjyrIlqzJWbjnRMN_PREVIEW_SMALL.webpI/WM-WorkerWrapper(28207): Worker result SUCCESS for Work [ id=99cc9c0a-64ce-4e62-beb5-70017e4c8ef5, tags={ taskId=3958078286, com.bbflight.background_downloader.DownloadWorker, BackgroundDownloaderPlugin, group=default } ]

Here the steps I took so far to debug the problem:

  1. I added custom logging to the native processStatusUpdate method inside the DownloadWorker class.

    fun processStatusUpdate(
        backgroundDownloadTask: BackgroundDownloadTask,
        status: DownloadTaskStatus
    ) {
        Log.i(
            TAG,
            "processStatusUpdate called"
         )
        if (backgroundDownloadTask.providesStatusUpdates()) {
            Log.i(
                    TAG,
                    "backgroundDownloadTask.providesStatusUpdates() is true"
            )
            Handler(Looper.getMainLooper()).post {
                try {
                    val gson = Gson()
                    val arg = listOf<Any>(
                        gson.toJson(backgroundDownloadTask.toJsonMap()),
                        status.ordinal
                    )
                    Log.i(
                            TAG,
                            "send statusUpdate to flutter"
                    )
                    BackgroundDownloaderPlugin.backgroundChannel?.invokeMethod(
                        "statusUpdate",
                        arg
                    )
                } catch (e: Exception) {
                    Log.w(TAG, "Exception trying to post status update: ${e.message}")
                }
            }
        }
        // if task is in final state, process a final progressUpdate and remove from
        // persistent storage
        if (status != DownloadTaskStatus.running) {
            when (status) {
                DownloadTaskStatus.complete -> processProgressUpdate(
                    backgroundDownloadTask,
                    1.0
                )
                DownloadTaskStatus.failed -> processProgressUpdate(backgroundDownloadTask, -1.0)
                DownloadTaskStatus.canceled -> processProgressUpdate(
                    backgroundDownloadTask,
                    -2.0
                )
                DownloadTaskStatus.notFound -> processProgressUpdate(
                    backgroundDownloadTask,
                    -3.0
                )
                else -> {}
            }
            BackgroundDownloaderPlugin.prefsLock.write {
                val jsonString =
                    BackgroundDownloaderPlugin.prefs.getString(
                        BackgroundDownloaderPlugin.keyTasksMap,
                        "{}"
                    )
                val backgroundDownloadTaskMap =
                    BackgroundDownloaderPlugin.gson.fromJson<Map<String, Any>>(
                        jsonString,
                        BackgroundDownloaderPlugin.mapType
                    ).toMutableMap()
                backgroundDownloadTaskMap.remove(backgroundDownloadTask.taskId)
                val editor = BackgroundDownloaderPlugin.prefs.edit()
                editor.putString(
                    BackgroundDownloaderPlugin.keyTasksMap,
                    BackgroundDownloaderPlugin.gson.toJson(backgroundDownloadTaskMap)
                )
                editor.apply()
            }
        }
    }

All three logs appear in the console on android.

  1. I added a setMethodCallHandler to the apps main method.

void main() { final widgetsBinding = WidgetsFlutterBinding.ensureInitialized(); FlutterNativeSplash.preserve(widgetsBinding: widgetsBinding);

await CrashReportingService.instance.init(); await Firebase.initializeApp();

MobileAds.instance.initialize();

FileDownloader.initialize( downloadStatusCallback: folderDownloadStatusCallback, );

MethodChannel('com.bbflight.background_downloader.background') .setMethodCallHandler((call) { print('received download background call: $call'); });

// ... some other code }

On ios the print statement is visible in the console, on android not.

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1399286243, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF5VCHZUE54TBTTCKDZPGN3WTQHB5ANCNFSM6AAAAAAUAA63RM . You are receiving this because you were mentioned.Message ID: @.***>

781flyingdutchman commented 1 year ago

Sorry our emails crossed. It sounds like it's a problem with how the method channel is set up. I'm not in a place where I can look things up but I'll do some research and get back to you.

Can you confirm you only start initialize and download after you call runApp? And is there any other plugin that might interfere with the establishment of a FlutterActivity in Android? You could try eliminating a few other plugins to see if there's one that causes the problem, if that's feasible.

On Sat, Jan 21, 2023, 11:21 AM Bram Bout @.***> wrote:

Thanks so much for your help debugging this. Can you add one more log statement just before the .invokeMethod call in Kotlin, to confirm that baxkgroundChannel is not null? If it is null, can you add logs to the onAttachedToEngine and onDetachedFromEngine calls to understand why it's not getting set? My current thinking is that that may be the issue, though I have to admit that I don't understand why this would happen, especially since it's working for you in the example app!

On Sat, Jan 21, 2023, 8:45 AM Gregor Weber @.***> wrote:

Hey I tried adding @pragma('vm:entry-point') but it did not change anything.

Here are the logs for a download

V/BackgroundDownloaderPlugin(28207): Starting task with id 3958078286 I/DownloadWorker(28207): Starting download for taskId 3958078286 I/DownloadWorker(28207): Successfully downloaded taskId 3958078286 to /data/user/0/de.knowunity.app/app_flutter/BQfgrjyrIlqzJWbjnRMN_PREVIEW_SMALL.webp I/WM-WorkerWrapper(28207) http://de.knowunity.app/app_flutter/BQfgrjyrIlqzJWbjnRMN_PREVIEW_SMALL.webpI/WM-WorkerWrapper(28207): Worker result SUCCESS for Work [ id=99cc9c0a-64ce-4e62-beb5-70017e4c8ef5, tags={ taskId=3958078286, com.bbflight.background_downloader.DownloadWorker, BackgroundDownloaderPlugin, group=default } ]

Here the steps I took so far to debug the problem:

  1. I added custom logging to the native processStatusUpdate method inside the DownloadWorker class.

    fun processStatusUpdate(
        backgroundDownloadTask: BackgroundDownloadTask,
        status: DownloadTaskStatus
    ) {
        Log.i(
            TAG,
            "processStatusUpdate called"
         )
        if (backgroundDownloadTask.providesStatusUpdates()) {
            Log.i(
                    TAG,
                    "backgroundDownloadTask.providesStatusUpdates() is true"
            )
            Handler(Looper.getMainLooper()).post {
                try {
                    val gson = Gson()
                    val arg = listOf<Any>(
                        gson.toJson(backgroundDownloadTask.toJsonMap()),
                        status.ordinal
                    )
                    Log.i(
                            TAG,
                            "send statusUpdate to flutter"
                    )
                    BackgroundDownloaderPlugin.backgroundChannel?.invokeMethod(
                        "statusUpdate",
                        arg
                    )
                } catch (e: Exception) {
                    Log.w(TAG, "Exception trying to post status update: ${e.message}")
                }
            }
        }
        // if task is in final state, process a final progressUpdate and remove from
        // persistent storage
        if (status != DownloadTaskStatus.running) {
            when (status) {
                DownloadTaskStatus.complete -> processProgressUpdate(
                    backgroundDownloadTask,
                    1.0
                )
                DownloadTaskStatus.failed -> processProgressUpdate(backgroundDownloadTask, -1.0)
                DownloadTaskStatus.canceled -> processProgressUpdate(
                    backgroundDownloadTask,
                    -2.0
                )
                DownloadTaskStatus.notFound -> processProgressUpdate(
                    backgroundDownloadTask,
                    -3.0
                )
                else -> {}
            }
            BackgroundDownloaderPlugin.prefsLock.write {
                val jsonString =
                    BackgroundDownloaderPlugin.prefs.getString(
                        BackgroundDownloaderPlugin.keyTasksMap,
                        "{}"
                    )
                val backgroundDownloadTaskMap =
                    BackgroundDownloaderPlugin.gson.fromJson<Map<String, Any>>(
                        jsonString,
                        BackgroundDownloaderPlugin.mapType
                    ).toMutableMap()
                backgroundDownloadTaskMap.remove(backgroundDownloadTask.taskId)
                val editor = BackgroundDownloaderPlugin.prefs.edit()
                editor.putString(
                    BackgroundDownloaderPlugin.keyTasksMap,
                    BackgroundDownloaderPlugin.gson.toJson(backgroundDownloadTaskMap)
                )
                editor.apply()
            }
        }
    }

All three logs appear in the console on android.

  1. I added a setMethodCallHandler to the apps main method.

void main() { final widgetsBinding = WidgetsFlutterBinding.ensureInitialized(); FlutterNativeSplash.preserve(widgetsBinding: widgetsBinding);

await CrashReportingService.instance.init(); await Firebase.initializeApp();

MobileAds.instance.initialize();

FileDownloader.initialize( downloadStatusCallback: folderDownloadStatusCallback, );

MethodChannel('com.bbflight.background_downloader.background') .setMethodCallHandler((call) { print('received download background call: $call'); });

// ... some other code }

On ios the print statement is visible in the console, on android not.

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1399286243, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF5VCHZUE54TBTTCKDZPGN3WTQHB5ANCNFSM6AAAAAAUAA63RM . You are receiving this because you were mentioned.Message ID: @.***>

The-Redhat commented 1 year ago
The-Redhat commented 1 year ago

I finally found the issue: https://github.com/firebase/flutterfire/issues/9689

781flyingdutchman commented 1 year ago

Wow that's crazy. Looks like that big has been around for a while without a good workaround. Did you manage to get the download callbacks to work properly?

On Sun, Jan 22, 2023, 12:58 AM Gregor Weber @.***> wrote:

I finally found the issue: firebase/flutterfire#9689 https://github.com/firebase/flutterfire/issues/9689

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1399433620, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF5VCHZMICLQG2EXAT4C6HTWTTZCFANCNFSM6AAAAAAUAA63RM . You are receiving this because you were mentioned.Message ID: @.***>

The-Redhat commented 1 year ago

Hey, yeah I removed the firebase messaging onBackgroundMessage handler and now it works 🎉

I will follow the issue on github, hopefully they will fix it soon. In the mean time it might make sense to add a short note to the Readme of this package to save other devs some pain.

The-Redhat commented 1 year ago

Applying the workaround mentioned here: https://github.com/firebase/flutterfire/issues/9689#issuecomment-1304491789 also solves the issue

781flyingdutchman commented 1 year ago

@The-Redhat thanks again for debugging this issue - quite the find! I've added a note to the readme as suggested. @Maqcel based on your description of the issue I think it's very likely that you're suffering from the same bug in the firebase plugin. I hope you can implement the work-around suggested above. With that, I'm going to close this issue - thanks again for your help!

The-Redhat commented 1 year ago

@781flyingdutchman the workaround I mentioned would need to be implemented in the package. I don't really see a downside to it. Do you consider using it in the package?

Maqcel commented 1 year ago

@781flyingdutchman Hi I've read all things you wrote thru the weekend and my problem is also tight to the onBackgroundMessage, I'll see in some time if removing this will help Edit. Yep downloads now works, still no progress calls :( I entirely removed impl of onBackgroundMessage because it was left as an future thing.

781flyingdutchman commented 1 year ago

@Maqcel if you're not getting progress calls, check that you have set the .progressUpdates field of every task you enqueue to statusChangeAndProgressUpdates. By default, tasks only emit status updates.

781flyingdutchman commented 1 year ago

@The-Redhat Is the workaround that the backgroundChannel field needs to be not static?

Maqcel commented 1 year ago

The problem is related to the event loop enqueue works fine (with callbacks) all good after onBackgroundMessage removal. Soled on my side

781flyingdutchman commented 1 year ago

@The-Redhat can you try a possible fix for the issue with onBackgroundMessage? Install the package using a git reference, like this:

  background_downloader:
    git:
      url: git@github.com:781flyingdutchman/background_downloader.git
      ref: firebug

It's a branch off the new 2.0 version (so you may have to make a minor adjustment to your code to accommodate the new retry logic) and it sets the backgroundChannel differently so it persists across multiple instantiations of the plugin. It works for the regular case, please let me know if it also addresses the issue for you with the Firebase plugin

The-Redhat commented 1 year ago

@781flyingdutchman yup the new branch works perfectly fine with the onBackgroundMessage enabled.

781flyingdutchman commented 1 year ago

Incorporated the fix in the latest V1 branch and in the current V2 (main) branch. Thanks again for your help!

781flyingdutchman commented 1 year ago

@The-Redhat if you have a moment, could you confirm if the latest version (2.1.1) still works properly with the Firebase onMethodcall handler? I made a small change where I 'reference count' the number of onAttachToEngine and onDetachFromEngine calls, and set the background channel to null when that counter drops to 0. I think it doesn't matter for the fix, and it ensures we properly release all resources.

The-Redhat commented 1 year ago

Hey yes I just tested it and can confirm it works. Thanks for the effort!

letisoft commented 10 months ago

Hello,

I am afraid that the static backgroundChannel made it worse. If app runs in more than one instance backgroundChannel is not working.

Please note I have app that is also using the share intent so thought android:launchMode="singleInstance/Task" some apps do start new instance and the backgroundChannel is not working - callbacks nor comming.

I am trying to change the code to get it working. Will keep you updated.

Best regards: V

letisoft commented 10 months ago

Hello,

I made the following change in code. Instead of single backgroundChannel I made a hash map of backgroundChannels, so I keep each backgroundChannel with its context and it now works.

Please note hash is also static, so I am not sure what is the diff (I hate kotlin).

BDPlugin.backgroundChannel?.invokeMethod( method, argList )

didn't throw any error, but onProgress never called.

after my change: var channel :MethodChannel? = BDPlugin.bgChannelsByTask[task.taskId];

it is OK, any idea?

Best regards: V

781flyingdutchman commented 10 months ago

Thanks - I'm a little worried about keeping Context around in a static variable, but open to exploring this. Can you create a PR?

letisoft commented 10 months ago

Hello,

Thanks allot for your quick replay! I am not keeping context in static variable.

Unfortunately I am not allowed to share my app code, but what I have:

App that uses share_intent plugin and uploads file using your plugin.

Some apps like google photos are OK as they are not starting new instance of the app, but some like google files (files by google) start a fresh instance (without the previous one being closed).

This seems to confuse the callback so changes I made:

1) onAttachedToEngine put the create new channel and store it in the static hashmap applicationContext = flutterPluginBinding.applicationContext conextID = System.currentTimeMillis().toString();//not bets option but I made it for a quick test bgChannelsByContext[conextID] = MethodChannel(flutterPluginBinding.binaryMessenger,"com.bbflight.background_downloader.background");

2) methodEnqueue() ....

add same channel by taskid bgChannelsByTask[task.taskId] = bgChannelsByContext[conextID]!!;

3) taksworker: use the channel by taksID var channel :MethodChannel? = BDPlugin.bgChannelsByTask[task.taskId]; if(channel != null){ channel?.invokeMethod(method, argList) }

seems to be working so far, though I am not sure why

Best regards: V

781flyingdutchman commented 10 months ago

I appreciate you can't share your app code, but if you make changes to the open source code (i.e. the plugin) then it's kinda expected that you contribute that back to the open source community. I'm going to leave it as is, as I don't fully understand what you're trying to do (and why).

letisoft commented 10 months ago

Hello,

Sorry I didn't explain it well as I was in a hurry.

Yes sure I will polish the changes today and I will submit pull request.

What I was saying is that I am not very good with kotlin so may be you will have to rework my changes. Also I am not sure why it makes a difference.

I hope I have more time today so I will try to creat a test app too.

Best regards: V

On Wed, 15 Nov 2023, 07:59 781flyingdutchman, @.***> wrote:

I appreciate you can't share your app code, but if you make changes to the open source code (i.e. the plugin) then it's kinda expected that you contribute that back to the open source community. I'm going to leave it as is, as I don't fully understand what you're trying to do (and why).

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1811855353, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGMOXKEZ62HL4IXFOJWMHDYERK3JAVCNFSM6AAAAAAUAA63ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJRHA2TKMZVGM . You are receiving this because you commented.Message ID: @.***>

781flyingdutchman commented 10 months ago

No worries, I am happy to help, at this moment I just don't quite understand what you're doing yet so a PR would definitely be helpful. Thanks!

letisoft commented 10 months ago

Hello,

All in there. Please lete know of anything else needed. I put all comments in the pull request

Best regards: V

On Wed, 15 Nov 2023, 09:46 781flyingdutchman, @.***> wrote:

No worries, I am happy to help, at this moment I just don't quite understand what you're doing yet so a PR would definitely be helpful. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1811956166, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGMOXLJ4LO5SWOYOGZQFE3YERXMVAVCNFSM6AAAAAAUAA63ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJRHE2TMMJWGY . You are receiving this because you commented.Message ID: @.***>

781flyingdutchman commented 10 months ago

@letisoft this has proven very complicated, but I think I have a solution that works for your situation, as well as the original one (that required to move to a static field to begin with, related to Firebase).

Could you try changing pubspec.yaml to:

  background_downloader:
    git:
      url: git@github.com:781flyingdutchman/background_downloader.git
      ref: bgchannel

and see if the functionality now works? I've also changed the notification launch intent settings, so now you should be able to set android:launchMode="standard". There are many log messages for now, to help debug any issues you may still run into.

Basically, it's your approach, but channels are stored by applicationContext.hashCode() so that multiple instantiations of the plugin with the same applicationContext do not lead to multiple background channels. That is required for the Firebase issue above. That requires 'reference counting' so we know when a background channel is truly no longer valid. If so, we the purge all references to it - also in the mapping of taskId to background channel. To get the background channel for a task, we first try to get it from the context (applicationContext or ActivityContext), and if we can't we get it from the taskId map (that you introduced) and if it has been purged there we end up with null, which then forces the use of the storeLocally logic so we can retrieve these missed updates when the app restarts.

letisoft commented 10 months ago

Hello,

Thank you very much. I am sorry after being sick I had lots to catch-up. I am back to this project next week and I will give it a try.

Best regards: V

On Sun, 10 Dec 2023, 07:38 781flyingdutchman, @.***> wrote:

@letisoft https://github.com/letisoft this has proven very complicated, but I think I have a solution that works for your situation, as well as the original one (that required to move to a static field to begin with, related to Firebase).

Could you try changing pubspec.yaml to:

background_downloader: git: url: @.***:781flyingdutchman/background_downloader.git ref: bgchannel

and see if the functionality now works? I've also changed the notification launch intent settings, so now you should be able to set android:launchMode="standard". There are many log messages for now, to help debug any issues you may still run into.

Basically, it's your approach, but channels are stored by applicationContext.hashCode() so that multiple instantiations of the plugin with the same applicationContext do not lead to multiple background channels. That is required for the Firebase issue above. That requires 'reference counting' so we know when a background channel is truly no longer valid. If so, we the purge all references to it - also in the mapping of taskId to background channel. To get the background channel for a task, we first try to get it from the context (applicationContext or ActivityContext), and if we can't we get it from the taskId map (that you introduced) and if it has been purged there we end up with null, which then forces the use of the storeLocally logic so we can retrieve these missed updates when the app restarts.

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1848865307, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGMOXJKXHM2KJW3WJA5KSDYIVDEPAVCNFSM6AAAAAAUAA63ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHA3DKMZQG4 . You are receiving this because you were mentioned.Message ID: @.***>

letisoft commented 9 months ago

Hello,

I am finally back to this project!

But when I tried I got this error: fatal: Could not read from remote repository.

Any idea why?

Best regards: V

On Sun, Dec 10, 2023 at 9:30 AM Violeta Dimitrova @.***> wrote:

Hello,

Thank you very much. I am sorry after being sick I had lots to catch-up. I am back to this project next week and I will give it a try.

Best regards: V

On Sun, 10 Dec 2023, 07:38 781flyingdutchman, @.***> wrote:

@letisoft https://github.com/letisoft this has proven very complicated, but I think I have a solution that works for your situation, as well as the original one (that required to move to a static field to begin with, related to Firebase).

Could you try changing pubspec.yaml to:

background_downloader: git: url: @.***:781flyingdutchman/background_downloader.git ref: bgchannel

and see if the functionality now works? I've also changed the notification launch intent settings, so now you should be able to set android:launchMode="standard". There are many log messages for now, to help debug any issues you may still run into.

Basically, it's your approach, but channels are stored by applicationContext.hashCode() so that multiple instantiations of the plugin with the same applicationContext do not lead to multiple background channels. That is required for the Firebase issue above. That requires 'reference counting' so we know when a background channel is truly no longer valid. If so, we the purge all references to it - also in the mapping of taskId to background channel. To get the background channel for a task, we first try to get it from the context (applicationContext or ActivityContext), and if we can't we get it from the taskId map (that you introduced) and if it has been purged there we end up with null, which then forces the use of the storeLocally logic so we can retrieve these missed updates when the app restarts.

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1848865307, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGMOXJKXHM2KJW3WJA5KSDYIVDEPAVCNFSM6AAAAAAUAA63ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHA3DKMZQG4 . You are receiving this because you were mentioned.Message ID: @.***>

letisoft commented 9 months ago

I have tested 8.0.2 I am afraid same issues. Switching back to the changes I made worked.

Please let me know if you need help?

Best regards: V

On Wed, Dec 20, 2023 at 4:30 PM Violeta Dimitrova @.***> wrote:

Hello,

I am finally back to this project!

But when I tried I got this error: fatal: Could not read from remote repository.

Any idea why?

Best regards: V

On Sun, Dec 10, 2023 at 9:30 AM Violeta Dimitrova @.***> wrote:

Hello,

Thank you very much. I am sorry after being sick I had lots to catch-up. I am back to this project next week and I will give it a try.

Best regards: V

On Sun, 10 Dec 2023, 07:38 781flyingdutchman, @.***> wrote:

@letisoft https://github.com/letisoft this has proven very complicated, but I think I have a solution that works for your situation, as well as the original one (that required to move to a static field to begin with, related to Firebase).

Could you try changing pubspec.yaml to:

background_downloader: git: url: @.***:781flyingdutchman/background_downloader.git ref: bgchannel

and see if the functionality now works? I've also changed the notification launch intent settings, so now you should be able to set android:launchMode="standard". There are many log messages for now, to help debug any issues you may still run into.

Basically, it's your approach, but channels are stored by applicationContext.hashCode() so that multiple instantiations of the plugin with the same applicationContext do not lead to multiple background channels. That is required for the Firebase issue above. That requires 'reference counting' so we know when a background channel is truly no longer valid. If so, we the purge all references to it - also in the mapping of taskId to background channel. To get the background channel for a task, we first try to get it from the context (applicationContext or ActivityContext), and if we can't we get it from the taskId map (that you introduced) and if it has been purged there we end up with null, which then forces the use of the storeLocally logic so we can retrieve these missed updates when the app restarts.

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1848865307, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGMOXJKXHM2KJW3WJA5KSDYIVDEPAVCNFSM6AAAAAAUAA63ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHA3DKMZQG4 . You are receiving this because you were mentioned.Message ID: @.***>

781flyingdutchman commented 9 months ago

Hmm, that's unfortunate. Can you share the log messages with - in between the log messages - a description of what you did? I am trying to understand what exactly happens when you create another instance, when you close that instance (or the original one) etc.

I appreciate that the modification you have made works for you, but it almost certainly will re-introduce the bug that started this issue thread to begin with, when using Firebase (and of course that is a widely used framework, so I can't break that). So, we need to find a way that works for your case without breaking the Firebase case.

781flyingdutchman commented 9 months ago

Hi, never mind my previous response. You shouldn't be testing 8.02 (that's a release) but this specific development version, by changing your pubspec.yaml entry to:

  background_downloader:
    git:
      url: git@github.com:781flyingdutchman/background_downloader.git
      ref: bgchannel

But, I also did more work on this and now have a different approach. The branch bgchannel has a modified example app that has a 'Spawn' button that spawns a new instance of the same app, and it works when I test it. I have also tested it with the FireBase issue and it works for me. Can you test the bgchannel branch with your setup, and let me know if this works?

letisoft commented 9 months ago

Hello,

This one seems to be working fine! I will keep testing and let you know if any issues are found.

I have two other questions: 1) on iOS notification is not shown if app is running, which is not the case when I show notifications using https://pub.dev/packages/flutter_local_notifications . Notification is there but it is not popping up 2) Queueing tasks - do you know if FileDownloader().enqueue(task) is supposed to execute tasks in the order they are queued or they will run in parale?

The problem I have is as follow: fille us uploaded and after upload is done there is an extra command I have to send to the server (kind of commit for the upload)

Best regards: V

On Thu, Dec 21, 2023 at 7:09 AM 781flyingdutchman @.***> wrote:

Hi, never mind my previous response. You shouldn't be testing 8.02 (that's a release) but this specific development version, by changing your pubspec.yaml entry to:

background_downloader: git: url: @.***:781flyingdutchman/background_downloader.git ref: bgchannel

But, I also did more work on this and now have a different approach. The branch bgchannel has a modified example app that has a 'Spawn' button that spawns a new instance of the same app, and it works when I test it. I have also tested it with the FireBase issue and it works for me. Can you test the bgchannel branch with your setup, and let me know if this works?

— Reply to this email directly, view it on GitHub https://github.com/781flyingdutchman/background_downloader/issues/6#issuecomment-1865489316, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGMOXJYTWPWGAQIB4UIUJ3YKPABTAVCNFSM6AAAAAAUAA63ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRVGQ4DSMZRGY . You are receiving this because you were mentioned.Message ID: @.***>

781flyingdutchman commented 9 months ago

Wonderful!

On iOS notifications, see here and especially the Apple guideline, which recommend against showing notifications while the app is in the foreground. I'm simply sticking to that guideline to be consistent with the experience that Apple users expect.

Actual execution of the tasks is done in parallel. If you want to manage execution order and parallellism you can use a task queue. But for your case, can't you just interview the TaskStatus.complete and send your commit message to the server then?

781flyingdutchman commented 9 months ago

Published as V8.0.4. Thanks for your help @letisoft