rollbar / rollbar-flutter

Rollbar for Dart and Flutter
https://docs.rollbar.com/docs/flutter
MIT License
21 stars 27 forks source link

Library is not reporting issues post 0.2.0 updates. #45

Closed matux closed 2 years ago

matux commented 2 years ago

Shortly after 0.2.0 was launched, we introduced the concept of a RollbarInfrastructure singleton which decides whether to persist the payload or send it using the HttpSender.

This is properly initialized by the UncaughtErrorHandler which is not the mechanism by which we catch unhandled errors (we do it through a Guard Zone initialized in our Flutter's library rollbar.dart).

Since the UncaughtErrorHandler works on its own Isolate (a Dart thread that shares no memory with the external world) and it doesn't catch errors, nothing should happen.

When the Guard Zone that catches uncaught errors, which is completely unrelated to UncaughtErrorHandler, catches an error, it tells our Rollbar class, the Rollbar class tells the CoreNotifier, the CoreNotifier tells the PersistentSender, the PersistentSender tells the RollbarInfrastructure, RollbarInfrastructure decides whether to persist it or not, and if not, it tells the HttpSender to send the error.

The bug exists at the point where the PersistentSender communicates with RollbarInfrastructure. At that point, since we're working on a separate Isolate thread (which remember, doesn't share memory), RollbarInfrastructure doesn't exists. Therefore, a new singleton is instantiated but not through the initialization mechanism that also initializes Late variables.

Since the Late variables haven't been initialized, the library throws an exception, which is silently caught by the try/catch boundary of PersistentSender and since the ModuleLogger doesn't work, it fails silently.

I surmise the problem is, as it usually is, OOP. :)

The hierarchy is very complicated and hard to reason about. The relation between the Senders, the CoreNotifier, the Infrastructure and the UncaughtErrorHandler doesn't properly delimit side-effect boundaries, which mutates library state all along their transactions, and it's very easy to miss errors.

On top of that, the singleton (which is evil) architecture allows for multiple ways of instantiating RollbarInfrastructure but only one is the correct non-crashy way.

This can be very easily reproduced by checking out https://github.com/rollbar/rollbar-flutter/commit/c0084204f16b10baf563ed534d8b74a4ba974b5c and placing a breakpoint on https://github.com/rollbar/rollbar-flutter/commit/c0084204f16b10baf563ed534d8b74a4ba974b5c#diff-c0a65f4aa5c9f3b62c0ffe6d247e93e77308c83449ea59c4403cdef1342b39e5R34