Cache manager crashes for keys where data == null

[nyvelius]

In flutter_cache_manager.dart on line 64, I have a key where the corresponding data value is null. This causes a crash. The cause is a link to an image that has been removed and returns 404.

_cacheData[key] = new CacheObject.fromMap(key, data)

In my case the error is caught further up:

[VERBOSE-2:dart_error.cc(16)] Unhandled exception:
'package:flutter/src/painting/image_stream.dart': Failed assertion: line 362 pos 12: '_codec != null': is not true.
#0 _AssertionError._doThrowNew (dart:core/runtime/liberrors_patch.dart:37:39)
#1 _AssertionError._throwNew (dart:core/runtime/liberrors_patch.dart:33:5)
#2 MultiFrameImageStreamCompleter._handleCodecReady (package:flutter/src/painting/image_stream.dart)
#3 _RootZone.runUnary (dart:async/zone.dart:1381:54)
#4 _FutureListener.handleValue (dart:async/future_impl.dart:129:18)
#5 Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:637:45)
#6 Future._propagateToListeners (dart:async/future_impl.dart:666:32)
#7 Future._completeError (dart:async/future_impl.dart:489:5)
#8 _SyncCompleter._completeError (dart:async/future_impl.dart:55:12)
#9 _Completer.completeError (dart:async/future_impl.dart:27:5)
#10 CachedNetworkImageProvider._loadAsync (package:cached_network_im<…>

[renefloor]

Yes, I can imagine this crashes when the map is null

  CacheObject.fromMap(String url, Map map) {
    this.url = url;
    _map = map;

    if (_map.containsKey(_keyTouched)) {
      touched = new DateTime.fromMillisecondsSinceEpoch(_map[_keyTouched]);
    } else {
      touch();
    }

    lock = new Object();
  }

[nyvelius]

I haven't looked further into the code, but it seems that null data values shouldn't be cached in the first place -- since there isn't anything to cache. Or as a second resort, such values should be easy to evict. For now I just forked the repo and added a quick workaround so I could keep working.

jsonCache.forEach((key, data) {
  if (data == null) {
    return;
  }
  _cacheData[key] = new CacheObject.fromMap(key, data);

[mig35]

Had the same issue. Above fix is a good solution. When do you think you will publish it?

[renefloor]

Sorry it took some time, but I wanted to fix it good. The data == null fix was only mitigating the effects of the problem. I just found time to have a good look at it and fixed a couple of problems I found. Can you confirm the problems are solved in 0.4.1?

[nyvelius]

@renefloor I did some testing and 0.4.1 is certainly an improvement, but there are some remaining issues.

1. With wifi turned ON, open a view with a list of images where some are off screen.
2. Turn wifi OFF.
3. Scroll down so images that were off screen are now visible.
4. Turn wifi back ON.

Result: The images do not load and will not load even when switching between screens or minimizing and re-opening the app.

Unlike before patch 0.4.1, they will now load if the app is completely closed and reopened.

[renefloor]

Yes true.
I also found that when an image fails to load and then you turn on a data connection you have to do some effort to get it reloaded. I will probably have to keep track of changes in the network connection after an image has failed loading. I will make a separate issue for that in the CachedNetworkImage library.

[nyvelius]

That could work. There is the connectivity package, but as the authors point out, detecting connectivity can be tricky:

Note that on Android, this does not guarantee connection to Internet. For instance, the app might have wifi access but it might be a VPN or a hotel WiFi with no access.

I don't know much about the internals of Flutter, but would it be possible to force a retry at least when re-opening a screen?

https://pub.dartlang.org/packages/connectivity

Edit: On second thought, maybe some sort of refresh functionality could be left up to the developer using the library? Something along the lines of creating a reference to the image cache and then calling refresh whenever it is appropriate, such as when internet connectivity is detected OR the user pulls down to refresh OR whatever works for that particular app.

[renefloor]

That is indeed a good idea as well. I am aware that checking whether you have an internet connection is impossible to do in general, but a change in the connectivity is a pretty good indicator that there could be a change in internet connection.
Leaving this to the developer gives him/her indeed more options on how this should happen, but I think it is better to do this within the library, at least to a certain point. Maybe we could make a default behaviour and an option to override that.

[marianoarga]

Hello, is there any update regarding this problem?