newrelic/newrelic-flutter-agent

GoRouter observer causes exception

Closed this issue · 22 comments

while adding
observers: <NavigatorObserver>[NewRelicNavigationObserver()],

to the go_router in my project :

an exception while navigation to one of my screens
:
package:flutter/src/widgets/navigator.dart': Failed assertion: line 2993 pos 18: '!navigator._debugLocked': is not true.

any help with this issue

═══════ Exception caught by widgets library ═══════════════════════════════════
The following NoSuchMethodError was thrown building InheritedGoRouter(goRouter: Instance of 'GoRouter'):
Class 'RouteSettings' has no instance getter 'key'.
Receiver: Instance of 'RouteSettings'
Tried calling: key

The relevant error-causing widget was:
MaterialApp MaterialApp:file:///Users/mohammedadelalisorour/Documents/My%20%20Work/Flutter/sevenSquare/the_laundry_hub/lib/app.dart:30:30

When the exception was thrown, this was the stack:
#0 Object.noSuchMethod (dart:core-patch/object_patch.dart:38:5)
object_patch.dart:38
#1 NewRelicNavigationObserver._addGoRouterBreadcrumb (package:newrelic_mobile/newrelic_navigation_observer.dart:83:30)
newrelic_navigation_observer.dart:83
#2 NewRelicNavigationObserver.didPush (package:newrelic_mobile/newrelic_navigation_observer.dart:45:9)
newrelic_navigation_observer.dart:45
#3 _NavigatorPushObservation.notify (package:flutter/src/widgets/navigator.dart:3338:14)
navigator.dart:3338
#4 List.forEach (dart:core-patch/growable_array.dart:416:8)
growable_array.dart:416
#5 NavigatorState._flushObserverNotifications (package:flutter/src/widgets/navigator.dart:4333:27)
navigator.dart:4333
#6 NavigatorState._flushHistoryUpdates (package:flutter/src/widgets/navigator.dart:4295:5)
navigator.dart:4295
#7 NavigatorState._updatePages (package:flutter/src/widgets/navigator.dart:4161:5)
navigator.dart:4161
#8 NavigatorState.didUpdateWidget (package:flutter/src/widgets/navigator.dart:3803:7)
navigator.dart:3803
#9 StatefulElement.update (package:flutter/src/widgets/framework.dart:5643:55)
.....
..
..

════════ Exception caught by Flutter framework ═════════════════════════════════
'package:flutter/src/widgets/navigator.dart': Failed assertion: line 2993 pos 18: '!navigator._debugLocked': is not true.
════════════════════════════════════════════════════════════════════════════════
D/newrelic(23857): PayloadController: 0ms. waiting to submit payload [8d20ef85-ac51-4a28-b6aa-c8093ea521dd].
D/newrelic(23857): HarvestTimer: time since last tick: 59997
D/newrelic(23857): Harvest: tick
D/newrelic(23857): Harvester state: CONNECTED
I/newrelic(23857): Harvester: connected
I/newrelic(23857): Harvester: Sending [8] HTTP transactions.
I/newrelic(23857): Harvester: Sending [0] activity traces.
I/newrelic(23857): Harvester: Sending [0] session attributes.
I/newrelic(23857): Harvester: Sending [0] analytics events.

when i remove the observer everything is working fine

@mohadel92 are you still seeing this issue?

same problem

Not the OP, but we're seeing a similar issue with using go_router for a modal bottom sheet.

When you use showModalBottomSheet, it can potentially push a route to the Navigator where the settings do not have a route path or name. For us, we're using showModalBottomSheet to pop up a widget, then when the user presses a button on that widget, we navigate to another page.

I believe the difference between Navigator and go_router when NewRelicNavigationObserver intercepts the route is that the Navigator path expects RouteSettings while the go_router path expects a widget.

image

@duraz0rz that's good feedback, i will check it from our side.

To add a bit more to this, we figured out a workaround on our end.

The docs for showBottomModalSheet state that you can pass routeSettings for NavigationObservers:

The optional settings parameter sets the RouteSettings of the modal bottom sheet sheet. This is particularly useful in the case that a user wants to observe PopupRoutes within a NavigatorObserver.

We didn't have this set when we were using Navigator and NewRelicNavigationObserver was not crashing, while it was crashing until we set routeSettings for a bottom sheet modal when we switched to go_router.

@duraz0rz @bernardoveras @mohadel92 can you check this in our latest flutter release?

Is happening again

new_relic: 1.0.8
go_router: 13.2.2
flutter: 3.19.2

Reopen the issue please

workaround:

class RouteSettingsExtends extends RouteSettings {
  const RouteSettingsExtends(
    this.key,
    this.child, {
    super.name,
    super.arguments,
  });
  final String key;
  final Widget child;
}
showModalBottomSheet<Widget>(
    context: context,
    routeSettings: RouteSettingsExtends(
      'Key',
      Container(),
      name: 'bla',
    ),
    child: Container(),
  );

@eugenio-tesio-ueno can you share example app or code snippet for this issue?

Hello @ndesai-newrelic , I hope you are feeling well.

This actually works for me by changing the child and key parameter that by default it does not bring in the routesettings.
before change:
code3

after change:
code

you know why you are looking for a child or key?.

It seems that the ModalBottomSheets use by default the RouteSettings class that has flutter with only the name, arguments property.

code2

@MiguelBelotto00 Agreed, let's switch to using 'name' instead of 'key'. Could you create a pull request for this change? I'll update the documentation to advise customers to include the 'name' parameter when using the Go router package for routing.

Yes, for sure I prepare the pr of this.

A good solution would be to stop using the _addGoRouterBreadcrumb and start using only the _addBreadcrumb but I don't know why there are two different functions.

code5

@eugenio-tesio-ueno can you share example app or code snippet for this issue?

@MiguelBelotto00 provided the error found #68 (comment)

@MiguelBelotto00 that's right, you can remove the method, now we are reading it from the name so we dont need different method.

Hello me again, another solution that works for me is directly using this.

code2

Reading a little more the code CustomTransitionPage is something from GoRouter, so I guess that's why you have two different functions.

This solution also works because the problem is the is RouteSettings since the base of the RouteSettings does not have a key or child as the MaterialPage or CustomTransitionPage does (The control is already done before using the _addGoRouterBreadcrumb function).i.e:

code4

Do you think this is a valid solution or do I just make the modification of using only the _addBreadcrumb function?

@eugenio-tesio-ueno i am not seeing that issue happening again, bottomsheet is modalBottomSheetRoute and we are not creating breadcrumb for that. if you are still seeing this issue please share example app with us.

Hey @MiguelBelotto00, it seems like route.child is providing the same value as name when passed. If you have any alternative suggestions, feel free to open a PR, and we'll review it.

If you take _addGoRouterBreadcrumb and cast fromRoute and toRoute as RouteSettings, you'll get a syntax error, like this:

image

RouteSetting class.

/// Data that might be useful in constructing a [Route].
@immutable
class RouteSettings {
  /// Creates data used to construct routes.
  const RouteSettings({
    this.name,
    this.arguments,
  });

  /// The name of the route (e.g., "/settings").
  ///
  /// If null, the route is anonymous.
  final String? name;

  /// The arguments passed to this route.
  ///
  /// May be used when building the route, e.g. in [Navigator.onGenerateRoute].
  final Object? arguments;

  @override
  String toString() => '${objectRuntimeType(this, 'RouteSettings')}(${name == null ? 'none' : '"$name"'}, $arguments)';
}

The workaround is to copy the NewRelicNavigationObserver and create your own observer. So I guess this issue isn't critical

@eugenio-tesio-ueno why are you casting this as route settings for from route and to route?

@eugenio-tesio-ueno why are you casting this as route settings for from route and to route?

To convert the dynamic fromRoute variable to RouteSettings type, (previously validated by the if condition) and to show you that key property does not exists.