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

GoRouter observer causes exception #68

Open
mohadel92 opened this issue Dec 18, 2023 · 22 comments
Open

GoRouter observer causes exception #68

mohadel92 opened this issue Dec 18, 2023 · 22 comments

Comments

@mohadel92
Copy link

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.

@mohadel92
Copy link
Author

when i remove the observer everything is working fine

@ndesai-newrelic
Copy link
Collaborator

@mohadel92 are you still seeing this issue?

@ndesai-newrelic
Copy link
Collaborator

@mohadel92 can you check this flutter/flutter#108544?

@bernardoveras
Copy link

same problem

@duraz0rz
Copy link

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

@ndesai-newrelic
Copy link
Collaborator

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

@duraz0rz
Copy link

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.

@ndesai-newrelic
Copy link
Collaborator

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

@eugenio-tesio-ueno
Copy link

eugenio-tesio-ueno commented Apr 12, 2024

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(),
  );

@ndesai-newrelic
Copy link
Collaborator

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

@MiguelBelotto00
Copy link

MiguelBelotto00 commented Apr 16, 2024

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

@ndesai-newrelic
Copy link
Collaborator

ndesai-newrelic commented Apr 16, 2024

@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.

@MiguelBelotto00
Copy link

MiguelBelotto00 commented Apr 16, 2024

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
Copy link

eugenio-tesio-ueno commented Apr 16, 2024

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

@MiguelBelotto00 provided the error found #68 (comment)

@ndesai-newrelic
Copy link
Collaborator

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

@MiguelBelotto00
Copy link

MiguelBelotto00 commented Apr 16, 2024

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?

@ndesai-newrelic
Copy link
Collaborator

@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.

@ndesai-newrelic
Copy link
Collaborator

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.

@eugenio-tesio-ueno
Copy link

eugenio-tesio-ueno commented Apr 17, 2024

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)';
}

@eugenio-tesio-ueno
Copy link

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

@ndesai-newrelic
Copy link
Collaborator

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

@eugenio-tesio-ueno
Copy link

eugenio-tesio-ueno commented Apr 18, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants