Wrap notification updates in try catch to avoid errors #199
Conversation
…re teardown on lower resource devices.
| Manager manager = new Manager(ContextUtil.getApplicationContext(), ref); | ||
| manager.send(notificationTitle, notificationText, progress, total); | ||
| try { | ||
| Context context = ContextUtil.getApplicationContext(); |
There was a problem hiding this comment.
If this was where the context issue was originating from then it probably has to do with removing the worker reference from this utility in combination with using some non-RemoteListenableWorker workers. Since the non-RemoteListenableWorker workers would be started by the system, we wouldn't necessarily have access to the foreground service. Storing static instances of the worker is bad, not only for the reasons regarding the context, but also because multiple instances could be running at once, so you don't necessarily get the instance you expect.
An alternative would be to go back to having all workers be RemoteListenableWorkers. Or use a sort of context manager approach in the worker such that this utility returns that context when called within that block
There was a problem hiding this comment.
I'm going to merge this regardless. I added a follow up issue
This seemed like the simplest way to handle these errors and stop them from breaking job execution, in local testing on a low powered Android 9 device, I confirmed that the errors were properly caught, and that tasks continued to run successfully. Also, notifications did not pile up and were properly cleared, in spite of errors.