Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] NetworkTileProvider has poorer performance and new unhandled exceptions since v6 #1698

Closed
vitalsh opened this issue Oct 20, 2023 · 46 comments · Fixed by #1742
Closed
Labels
bug This issue reports broken functionality or another error P: 2 (soon™?)
Milestone

Comments

@vitalsh
Copy link

vitalsh commented Oct 20, 2023

What is the bug?

Sometimes not all the tiles are being loaded/drawn.
After moving/dragging the map all the tiles are immediately drawn/refreshed

How can we reproduce it?

It might be me with all the code in my project, but try with these tiles:
TileLayer(
urlTemplate: "https://{s}.basemaps.cartocdn.com/rastertiles/voyager/{z}/{x}/{y}.png",
subdomains: ['a', 'b', 'c', 'd'],
tileProvider: NetworkTileProvider(),
keepBuffer: 10,
maxZoom: 23,
retinaMode: false);

Do you have a potential solution?

A workaround: add a method in MapController to refresh/redraw the tiles on demand.

Platforms

All

Severity

Obtrusive: Prevents normal functioning but causes no errors in the console

@vitalsh vitalsh added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels Oct 20, 2023
@vitalsh
Copy link
Author

vitalsh commented Oct 20, 2023

flutter_map_tiles_not_loading

@JaffaKetchup
Copy link
Member

Can I ask what version you are running?

@vitalsh
Copy link
Author

vitalsh commented Oct 20, 2023

flutter_map: ^6.0.0
flutter: 3.13.7/8
dart: 3.1.4

@JaffaKetchup
Copy link
Member

Does the issue occur with the OSM tile server? (Note: only use the OSM tile server for testing.)

@vitalsh
Copy link
Author

vitalsh commented Oct 20, 2023

I don't use OSM (I use https://{s}.basemaps.cartocdn.com/rastertiles/voyager/{z}/{x}/{y}.png).
Can try OSM and update.

@mattmyne
Copy link

mattmyne commented Oct 21, 2023

I don't know if it's related but I'm running a local OSM server and having updated to flutter map v6 have noticed tiles take longer to display than with flutter map v5. I'm testing on Windows 11 native. Particularly when quickly zooming in and out, grey tiles are visible for a second or so in many places, when previously it would be almost instant (given local server). Nothing's changed on the server or it's caching.

@swust-xl
Copy link

I got the same problem.I upgraded flutter_map from V4 to V6, then the console showed ImageCodecException :Failed to detect image file format using the file header. File header was [0x3c 0x21 0x44 0x4f 0x43 0x54 0x59 0x50 0x45 0x20].
I found it was because the tile requested that didn't exist on Server or Assets.In V4 it throws a 404 NOT FOUND error,but in V6 it throws a ImageCodecException . I don't know in V6 why it always gave a strange fixed byte data when the tile request doesn't exist on server .To solve this problem just restrict CameraConstraint . But even the console shows no error log, some tiles are still not shown correctly just like @vitalsh 's problem.

@manlyman29
Copy link

On a related note, the flutter map throws an exception each time it can not load a tile, and that makes debugging applications with a flutter_map widget, too troublesome. It constantly throws Connection closed before header was received, Socket Exception, and similar server-related errors and pauses the debug session. A fix or workaround for this would be great. This is happening in both OSM and MapBox tile servers.

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Oct 23, 2023

@manlyman29 Are you also using the standard tile provider? What version are you running, and does it occur in earlier versions?

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Oct 23, 2023

@swust-xl That sounds possibly like a different problem.

(Deleted out of office response from you)

@fleaflet fleaflet deleted a comment from swust-xl Oct 23, 2023
@mattmyne
Copy link

mattmyne commented Oct 23, 2023

On a related note, the flutter map throws an exception each time it can not load a tile, and that makes debugging applications with a flutter_map widget, too troublesome. It constantly throws Connection closed before header was received, Socket Exception, and similar server-related errors and pauses the debug session. A fix or workaround for this would be great. This is happening in both OSM and MapBox tile servers.

To confirm I also get this if I just stop my local OSM server. Every time flutter map tries to load tiles (startup, move, zoom, etc.) I get something similar to this on the VS Code debug console:

flutter: (OS Error: The remote computer refused the network connection.

flutter: , errno = 1225), address = localhost, port = 64972, uri=http://localhost:8080/tile/18/130142/87058.png flutter: ClientException with SocketException: The remote computer refused the network connection.

At various times (I don't think every move, but most?) I also get:

══╡ EXCEPTION CAUGHT BY IMAGE RESOURCE SERVICE ╞════════════════════════════════════════════════════ The following _ClientSocketException was thrown resolving an image codec: ClientException with SocketException: The remote computer refused the network connection. (OS Error: The remote computer refused the network connection. , errno = 1225), address = localhost, port = 64838, uri=http://localhost:8080/tile/3/4/3.png

When the exception was thrown, this was the stack: #0 IOClient.send (package:http/src/io_client.dart:119:7) <asynchronous suspension> #1 RetryClient.send (package:http/retry.dart:115:20) <asynchronous suspension> #2 BaseClient._sendUnstreamed (package:http/src/base_client.dart:93:32) <asynchronous suspension> #3 BaseClient.readBytes (package:http/src/base_client.dart:58:22) <asynchronous suspension> #4 FlutterMapNetworkImageProvider._loadAsync (package:flutter_map/src/layer/tile_layer/tile_provider/network_image_provider.dart:84:11) <asynchronous suspension>

URL: http://localhost:8080/tile/3/4/3.png Fallback URL: null Current provider: FlutterMapNetworkImageProvider() ════════════════════════════════════════════════════════════════════════════════════════════════════

...followed by lots of:

Another exception was thrown: ClientException with SocketException: The remote computer refused the network connection.

I don't recall the behaviour in version 5, but I don't remember having this stream of exceptions each time!

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Oct 23, 2023

I'm not sure why you didn't see these before, but that's normal (possibly if you were using a remote URL before it kept retrying or never received a proper response somehow, but now you're on localhost your machine knows there's nothing there)!
If you use a NetworkImage with an unreachable URL, it spits out a similar message as far as I can remember.

We MUST return an image or an error from inside an image provider. We could return a transparent tile instead of an error, but that would just hide the problem and potentially cause more issues. I guess the way to solve it now that we're using logger elsewhere is to create a custom error log, but then we're back to the same issue.

An unreachable URL or 404 response causing errors in console is OK. A 404 error attempting and failing to be decoded (invalid codec) as an image is not OK, and probably means we have a logic gap in our custom image provider - but I'll need to check that.

@mootw
Copy link
Collaborator

mootw commented Oct 23, 2023

the custom image provider has historically thrown exceptions for network errors correct. in my app i use a custom tile provider that caches the tiles locally on disk. i need to re-write it soon with the new image decoder, but at the moment it also throws exceptions for network errors. , which can be annoying while debugging.

mine explicitly throws for all non-ok status codes. I believe my custom network provider is in the docs somewhere as the caching tiles example?

if (response.statusCode != HttpStatus.ok) {
        throw NetworkImageLoadException(
            statusCode: response.statusCode, uri: url);
      }

@manlyman29
Copy link

manlyman29 commented Oct 23, 2023

@JaffaKetchup Thanks for the reply, Yes it occurs on the standard NetworkTileProvider, with v6.0.0
I am not completely sure about earlier versions, I think this used to happen but a lot less.
Most of the time exception is from here:
flutter_map

Edit: Just noticed the CancellableNetworkTileProvider, Similar issue of constantly throwing an exception DioException [request cancelled]: The request was cancelled.

@mattmyne
Copy link

mattmyne commented Oct 23, 2023

I see what you're saying regarding the necessity of warning the user/program. I wonder if there's a way of reducing the frequency somehow though. Maybe after 10 tile exceptions in a row pause for a bit (and have a comment saying so?). It's just it seems while one exception/error is helpful, a massive stream of them is almost the opposite. I understand it's important to pick up cases where only one or two aren't loading, so a minimum threshold and pause might do both?

Maybe this isn't a priority as most people's URLs will be fine and they'll have a net connection, but thought I'd share my view given I'm getting notifications for this issue anyway 😉

@JaffaKetchup
Copy link
Member

We could possibly return a custom image that just is red and has the text "Failure", but that seems like a lot of effort, and at that point, we should bring back the failure image argument. It also seems a bit OTT, but maybe that's the best option. Let me know what you think.

We could also add an argument to the provider to just ignore errors and return a transparent tile - and maybe combining this with the above might work.

(I know it's not always helpful, but I always use "Run Without Debugging" from VS Code. Contrary to the name, all of Flutter's DevTools are available, along with hot reload/restart, it just doesn't support breakpoints, and errors are just left in console.)

The reason it may be appearing more is because we reinforced our error catching in this region, as it was ignoring some errors before, which caused some issues. Possibly, it's because we're now throwing every error caught inside the catchError block (which didn't exist before), rather than rethrowing (because we can't inside catchError).


The comment #1698 (comment) from above about a decoding error with 404 seems to be a different issue. With the changes to error handling in this area, we are now catching async errors directly from decode, rather than just using try/catch. Essentially, we're catching the error at decode time rather than response time, although I'm not entirely sure why this happens with some URLs and not others.


I've rewritten the _loadAsync method to be a little more flexible, reduce duplication, and stop mixing sync & async code:

 httpClient
    .readBytes(
      Uri.parse(useFallback ? fallbackUrl ?? '' : url),
      headers: headers,
    )
    .then((bytes) => ImmutableBuffer.fromUint8List(bytes))
    .then((buffer) => decode(buffer))
    .catchError((dynamic err) {
  // ignore: only_throw_errors
  if (useFallback || fallbackUrl == null) throw err as Object;
  return _loadAsync(key, chunkEvents, decode, useFallback: true);
});

I can't see a difference, but maybe there'll be a difference for some people.
It's also worth noting that this is more customizable. Rather than using a catch-all catchError, we could use the onError callback inside each then to handle each error more specifically - but I can't really see the point in that for now.

Open to other suggestions to change the way this works.

@JaffaKetchup
Copy link
Member

Of course, the ideal method would be to copy the implementation of NetworkImage closely, but that does involve a separate implementation for IO and web. I'll see if I can do it.

@mootw
Copy link
Collaborator

mootw commented Oct 23, 2023

the readBytes function returns a ClientException if the status code is not success, so that would explain why the 404 status codes are getting thrown up

@JaffaKetchup
Copy link
Member

Yeah, but I thought we had that in earlier version tbh.
Turns out, copying Flutter's network image implementation is quite difficult as they go quite deep into the behind-the-scenes implementations, particularly on the web.

@JaffaKetchup
Copy link
Member

@mattmyne @manlyman29 @swust-xl Please use #1703 to discuss anything about error handling. This issue should solely be used for its original purpose about reporting a performance issue with the default image provider.

@JaffaKetchup JaffaKetchup changed the title Not all tiles loaded/shown sometimes [BUG] Default TileProvider/ImageProvider has potentially poor performance and occasionally fails to fetch tiles Oct 23, 2023
@robiness

This comment was marked as off-topic.

@JaffaKetchup

This comment was marked as off-topic.

@LeeMatthewHiggins
Copy link

LeeMatthewHiggins commented Oct 25, 2023

I have the same issue, using mapbox static tiles, I have struggled to find the cause. I have tried the cancel tile provider still get the issue, lots of connection issues with sockets closed. I´m using the animated controller plugin to animate the map to positions. I suspect that its triggering a bunch of tile requests as it animates, and if the view tree is rebuilt it triggers even more. I even get out of file descriptors, which points to lots (and lots) of open connections. This is happening mainly on iOS (ios 17, iphone 13 pro max) also happens on on android (8.1.0 one plus), seems ok from a cold start, but after a few mins of use it starts to have the issue.

@TesteurManiak
Copy link
Collaborator

TesteurManiak commented Oct 25, 2023

@LeeMatthewHiggins if you think your issue comes from flutter_map_animations I invite you to check this issue to see how you can add a TileUpdateTransformer TesteurManiak/flutter_map_animations#13 (comment)

@robpot95
Copy link

robpot95 commented Oct 26, 2023

I would like to add input, I have noticed same problems with latest version of flutter maps. V5 the map was loading fine and with good performance. Now sometimes it does not load the area and it feels bit choppy when it try to load the areas. Hopefully a fix is soon on it's way 😕

@TesteurManiak
Copy link
Collaborator

@LeeMatthewHiggins Have you also correctly set the customId property as mentioned on the issue? TesteurManiak/flutter_map_animations#13 (comment)

@LeeMatthewHiggins
Copy link

I just removed the check to turn it on and off so its always on. I see the print statements Loading tiles etc.
Screenshot 2023-11-02 at 10 45 53

@TesteurManiak
Copy link
Collaborator

@LeeMatthewHiggins this is not what I was asking, have you specified the customId property to the "animated" method you are calling. Here's the example mentioned in the comment of the issue:

static const _useTransformerId = 'useTransformerId';

 GestureDetector(
  onTap: () => _animatedMapController.animateTo(
    dest: point,
    customId: _useTransformer ? _useTransformerId : null, // Here
  ),
  // ...
)

@LeeMatthewHiggins
Copy link

I have not added a custom id to the animation calls, I thought that was only used to trigger the custom behaviour or not, i just changed the transformer to always use the custom method. I do see the print statements and I do see it loading the destination tiles sooner than when not using it. Does the custom Id also do somthing else internally? the link you sent, the issue was that he still had the code that skips the custom behaviour if the custom id is not set (and he didn´t set it). But I just removed that code so it always does it (see above). It does make a improvement, but in my case its still not enough on 3G. If i just completey remove the animations then its usable and very rarely (if at all) do I get blank tiles. I will have one more try later today to see if I can get it working with the animation, i will add the custom Id. However this all seems very hacky to me.

@TesteurManiak
Copy link
Collaborator

TesteurManiak commented Nov 2, 2023

Yes the custom id is needed to use the transformer otherwise the transformer won't be able to identify a movement animation to fetch only the destination's tiles. You might want to re-check the full example to better understand the mechanisms induced in the transformer.

@LeeMatthewHiggins
Copy link

OK I have tried this out again, as I really want nice animations. I added the customId. However I do get tiles not loading. Its like the end frame that clips the tiles is not correct, you can see in this video that the tiles are loaded, you just have to move the map to have them pop in. The Tile update transformer does solve the massive amount of requests, but has this other issue that i actually think is just clipping the tiles rather than no loading, as they seem to instantly appear if you move the map manually. heres what I get...

Video

@LeeMatthewHiggins
Copy link

LeeMatthewHiggins commented Nov 6, 2023

Small update, I have refactored my app to make it as simple as possible, removed all flutter animations that would resize the flutter map widget or make it rebuild. I fix the map size so there is no movement. Seemed to work great. however I got this (see link video). Which now is exactly the same issue as the orginal issue reporter had. It doesn´t happen all the time, but when it does the same tiles never load in that session. if you move the map then others load. So i think there is maybe an issue with the way the system handles errors (maybe caching the error).

Video

@MendleM
Copy link

MendleM commented Nov 15, 2023

Would love to see this issue resolved. Tiles don't load on the initial load sometimes. I notice this happens at specific zoom levels from initial load. So if I zoom in slightly it will show tiles but as soon as I zoom back out to the zoom level of the initial load, those initial failed to load tiles are still missing.

Using CancellableNetworkTileProvider FYI.

@MendleM
Copy link

MendleM commented Nov 15, 2023

I've implemented a retry mechanism into the CancellableNetworkTileProvider which seems to fix it for now:

import 'dart:async';
import 'dart:ui';

import 'package:dio/dio.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_map/flutter_map.dart';

class CancellableNetworkTileProvider extends TileProvider {
  CancellableNetworkTileProvider({super.headers}) : _dio = Dio();

  final Dio _dio;

  @override
  bool get supportsCancelLoading => true;

  @override
  ImageProvider getImageWithCancelLoadingSupport(
    TileCoordinates coordinates,
    TileLayer options,
    Future<void> cancelLoading,
  ) =>
      _CNTPImageProvider(
        url: getTileUrl(coordinates, options),
        fallbackUrl: getTileFallbackUrl(coordinates, options),
        tileProvider: this,
        cancelLoading: cancelLoading,
        maxRetryCount: 3, // Set the maximum number of retries
      );

  @override
  void dispose() {
    _dio.close();
    super.dispose();
  }
}

class _CNTPImageProvider extends ImageProvider<_CNTPImageProvider> {
  final String url;
  final String? fallbackUrl;
  final CancellableNetworkTileProvider tileProvider;
  final Future<void> cancelLoading;
  final int maxRetryCount;

  const _CNTPImageProvider({
    required this.url,
    required this.fallbackUrl,
    required this.tileProvider,
    required this.cancelLoading,
    required this.maxRetryCount,
  });

  @override
  ImageStreamCompleter loadImage(
    _CNTPImageProvider key,
    ImageDecoderCallback decode,
  ) {
    final chunkEvents = StreamController<ImageChunkEvent>();

    return MultiFrameImageStreamCompleter(
      codec: _loadAsync(key, chunkEvents, decode, maxRetryCount),
      chunkEvents: chunkEvents.stream,
      scale: 1,
      debugLabel: url,
      informationCollector: () => [
        DiagnosticsProperty('URL', url),
        DiagnosticsProperty('Fallback URL', fallbackUrl),
        DiagnosticsProperty('Current provider', key),
      ],
    );
  }

  @override
  Future<_CNTPImageProvider> obtainKey(
    ImageConfiguration configuration,
  ) =>
      SynchronousFuture<_CNTPImageProvider>(this);

  Future<Codec> _loadAsync(
    _CNTPImageProvider key,
    StreamController<ImageChunkEvent> chunkEvents,
    ImageDecoderCallback decode,
    int remainingRetries, {
    bool useFallback = false,
  }) async {
    final cancelToken = CancelToken();
    unawaited(cancelLoading.then((_) => cancelToken.cancel()));

    try {
      final response = await tileProvider._dio.get<Uint8List>(
        useFallback ? fallbackUrl! : url,
        cancelToken: cancelToken,
        options: Options(
          headers: tileProvider.headers,
          responseType: ResponseType.bytes,
        ),
      );

      return decode(await ImmutableBuffer.fromUint8List(response.data!));
    } on DioException catch (e) {
      if (CancelToken.isCancel(e)) {
        return decode(
          await ImmutableBuffer.fromUint8List(TileProvider.transparentImage),
        );
      }
      if (remainingRetries > 0 && !useFallback) {
        // Retry the request
        return _loadAsync(key, chunkEvents, decode, remainingRetries - 1);
      } else if (fallbackUrl != null && !useFallback) {
        // Try loading from the fallback URL
        return _loadAsync(key, chunkEvents, decode, maxRetryCount, useFallback: true);
      }
      rethrow;
    } catch (_) {
      if (remainingRetries > 0 && !useFallback) {
        // Retry the request
        return _loadAsync(key, chunkEvents, decode, remainingRetries - 1);
      } else if (fallbackUrl != null && !useFallback) {
        // Try loading from the fallback URL
        return _loadAsync(key, chunkEvents, decode, maxRetryCount, useFallback: true);
      }
      rethrow;
    }
  }

  @override
  bool operator ==(Object other) =>
      identical(this, other) ||
      (other is _CNTPImageProvider && url == other.url && fallbackUrl == other.fallbackUrl);

  @override
  int get hashCode => Object.hashAll([url, fallbackUrl]);
}

@cpuell
Copy link

cpuell commented Nov 16, 2023

I want to add some findings.. i got this error or tile loading behaviour a lot when i use a flutter map location marker and the map has not the same initial lat, lng as my current location. Meaning, my location marker is my current position, but the map starts with a default position somewhere else.

see my sample widget here:

// Check doc https://pub.dev/packages/flutter_map_location_marker

  Widget myCurrentLocation() {
    return CurrentLocationLayer(
      followOnLocationUpdate: FollowOnLocationUpdate.always,
      turnOnHeadingUpdate: TurnOnHeadingUpdate.never,
      style: const LocationMarkerStyle(
        marker: DefaultLocationMarker(
          child: Icon(
            Icons.navigation,
            color: Colors.white,
          ),
        ),
        markerSize: Size(40, 40),
        markerDirection: MarkerDirection.heading,
      ),
    );
  }

@H585001
Copy link

H585001 commented Nov 30, 2023

To add to this annoying issue, i get ClientException: Connection closed while receiving data breakpoint for every unloaded tile when closing the map screen containing FlutterMap through the android back button, or the back button in the app bar. Seems to happen mostly when closing while tiles are being loaded, however sometimes happens when all tiles are visible on screen. Also, when closing the map screen before any tiles are loaded, a ClientException with SocketException: Connection attempt cancelled breakpoint is hit once for every tile. All breakpoints are triggered from the _loadAsync method of the FlutterMapNetworkImageProvider class. Also sometimes tiles don't load at all as described in the issue

@fleaflet fleaflet deleted a comment from swust-xl Nov 30, 2023
@JaffaKetchup
Copy link
Member

Related (also to #1703):

I'm not 100% sure why this happens - but I'm not sure there's so much we can do in regards to sometimes failing to fetch tiles. HTTP IO is a bit of a mess in Flutter/Dart.

Just check that this All Exceptions box is disabled for starters:
image

For example, with it enabled, I get this strange error - but the tiles still load perfectly fine afterward:
image


In terms of performance, I'll see what I can do, but I'm not confident there is so much. I will simplify as I mentioned in #1698 (comment), hopefully that will help a bit.

@JaffaKetchup
Copy link
Member

In terms of the issues with early HTTP client cancellation, that will be caused when the TileProvider providing the tiles is destroyed (when the TileLayer is destroyed) but still trying to load tiles. We'll see whether we can do anything about this.

@JaffaKetchup
Copy link
Member

JaffaKetchup commented Dec 1, 2023

#1742 should hopefully fix as much as we can of this. We're now following much closer to what Flutter documents for implementers of an ImageProvider - although there are still some differences.
For those using CancellableNetworkTileProvider, follow fleaflet/flutter_map_cancellable_tile_provider#4 & expect it in v2.

It definitely fixes the "ClientException: Connection closed while receiving data" and "ClientException: Client already closed" issues. There's also now a silenceExceptions argument for TileProviders, that will silence all issues and just return empty transparent tiles.

@JaffaKetchup JaffaKetchup changed the title [BUG] Default TileProvider/ImageProvider has potentially poor performance and occasionally fails to fetch tiles [BUG] NetworkTileProvider has poorer performance and new unhandled exceptions since v6 Dec 1, 2023
@josxha josxha modified the milestones: v6.1, v7.0 Dec 2, 2023
@saropa
Copy link

saropa commented Feb 23, 2024

Hi all, just adding a note that this PR 1742 (#1742) did fix our "connection attempt cancelled" (grey tiles) issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error P: 2 (soon™?)
Projects
Archived in project