Allow already initialized Sentry instances to be used in other packages

Problem Statement

Description

The current static only approach doesn’t gracefully allow other packages to rely (inject) on a Sentry instance that has already been initialized.

Such scenario can be seen in application packages (refer to the example Use case below), and also within other Sentry packages, as in package:sentry_flutter or as outlined by @uemann in a discussion. Some workarounds require to ignore the warning // ignore: invalid_use_of_internal_member, however, ignoring internal members is discouraged and is not the best way to resolve the issue. I would like the Sentry package to support such use cases gracefully without having to work around it or ignore warnings.

Relevant discussion:

https://github.com/getsentry/sentry-dart/discussions/2073

Use case

In occasions you may have an Application Package (as defined in the Dart documentation) that depends directly on two packages: package:flutter_sentry and a Dart Package that depends on package:sentry (see illustration).

sentry-diagram

The Dart Package does not depend on Flutter. The Dart Package has classes and methods that rely on a package:sentry . These methods abstract away package:sentry functionalities, for example captureEvent . The Dart Package wraps around them, includes additional logic and ensures that the Application Package doesn’t have a strong dependency with Sentry (meaning it can be swapped out by another Dart Package that relies on a different application performance monitoring [APM] provider easily when implementing a common interface). The illustration below shows how the application would easily swap out the Sentry dependency for another APM provider. For more information about the pattern refer to the Very Good Flutter architecture blog post.

sentry-diagram-swap

The Dart Package does not initialize Sentry, the Application Package does. The static Sentry instance that has been initialized in the Application Package can’t be passed as an argument to the Dart Package, making the following obviously not possible:

// package:my_app
void main() {
  // ...
  await Sentry.init((options) => options.dsn = 'dsn');
  final apmClient = MySentryClient(client: Sentry)
  // ...
}

// package:my_another_apm
class MySentryClient implements MyApmClient {
  const MySentryClient({
    required this.client,
  })

  final Sentry client;

  // ...
 }

Instead, it is preferred that the Dart Package gets injected a SentryClient or a Hub so that it can access its useful methods, for example:

// package:my_app
void main() {
  // ...
  await Sentry.init((options) => options.dsn = 'dsn');
  final sentryClient = SentryClient(Sentry.currentHub.options);
  final apmClient = MySentryClient(client: sentryClient)
  // ...
}

// package:my_another_apm
class MySentryClient implements MyApmClient {
  const MySentryClient({
    required this.client,
  });

  /// The Sentry client.
  final SentryClient client;

  // ...
 }

However, the above will report an invalid_use_of_internal_member. Meaning that the above usage is “invalid” and is ultimately discouraged. According to @ueman it is currently protected so:

[…] other Sentry libraries (files, dio, etc) to read from the options since they can't be initialized during the Sentry init call.

One could pass callbacks for each Sentry static method, as such:

// package:my_app
void main() {
  // ...
  await Sentry.init((options) => options.dsn = 'dsn');
  final sentryClient = SentryClient(Sentry.currentHub.options);
  final apmClient = MySentryClient(
  captureEvent: Sentry.captureEvent,
    addBreadcrumb: Sentry.addBreadcrumb,
  );
  // ...
}

However, the above is largely inconvenient since it will require the developer to define the function signatures to be injected in MySentryClient for each method, increasing the maintenance burden for future updates compared to simply relying on the SentryClient or Hub that already provide such methods without having to specify the signatures of each method.

Solution Brainstorm

Draft proposals

Below some changes that might help solve the above inconvenience:

Define a view only (immutable) SentryOptions (similar to those found in dart:collection, for example UnmodifiableListView) so that accessing the options outside the init method can be done safely. Then make a getter so that it is publicly accessible, not protected. The same view-only approach might apply to other relevant classes for this to be plausible.
Refactor the package to stop using a static Sentry to perform operations. In essence, the current approach introduces:
- A global variable (can’t be properly scoped) and developers can’t rely on their own preferred dependency injection tools to access Sentry .
- It is somewhat against the Effective Dart guideline (when only considering its public API) and its matching lint rule avoid_classes_with_only_static_members.
- It makes it cumbersome to benefit from the object oriented paradigm (which Dart is great at), since static methods make it purely functional.
Open to any other solutions that resolve the described inconvenience

Are you willing to submit a PR?

None

You can use the static classes of Sentry without initializing it in the package, though. If the application properly initializes Sentry, the events from the package are properly send.

So the following example

// in application code
SentryFlutter.init(...);

// in package code
Sentry.captureException(...);

works and reports the errors properly to Sentry. Sentry does all of the required things internally.

That's also how the other Sentry packages work. They simply use the static methods internally. They only need the options for configuration, not for reporting of the actual exceptions/performance traces etc. So in theory, the other Sentry packages could work without having access to the Sentry options.

Even Sentry itself uses the static methods internally: https://github.com/getsentry/sentry-dart/blob/e6b16cd4aa51b0236a52f4c6380d53b6d1a1aee9/dart/lib/src/hub_adapter.dart

If you need testability, I would recommend writing something like the HubAdapter (as mentioned above) yourself.

Hi @ueman thanks for the quick reply 💙 ! The information helps! I've left some questions based on your information, hopefully you can provide some clarity on those.

You can use the static classes of Sentry without initializing it in the package

That's good to know. I've added documentation in #2093 around such behaviour (since I wasn't sure if it was purposely designed for so, or if it was a discouraged practice) and added some documentation around the information you shared on #2073.

If you need testability, I would recommend writing something like the HubAdapter (as mentioned above) yourself.

Is there any documentation around testing Sentry for Dart/Flutter? What is the preferred way to verify that calls to Sentry (such as, Sentry.captureEvent) get called without really sending it to the Sentry servers?

I can't find testing related documentation on Flutter Sentry docs or Dart Sentry docs. It seems that Node has some, is there any tracking effort to document and improve the testing experience for Flutter and Dart?

With the current approach, what happens when you would like to have two Sentry instances (with different options) that you would like to report information to? How would you report captured events for one and not the other?

What happens if a package initialised Sentry to collect information, the application also initialises another Sentry to collect another information, and there is another package that would like to rely on the Sentry that got initialised in the application (not the one from the first package)? What would the static Sentry members (in the latter package) rely on, would it work as expected?

Here's some code to exemplify the above:

// in package B
Sentry.init(...);

// in application code
SentryFlutter.init(...); // different DSN

// in package C
Sentry.captureException(...);

I'm also curious to know what is the motivation for making everything static and global rather than letting the user have an actual instance?

I'm guessing that the "static everything" approach might be a consequence of trying to make the package API surface homogeneous throughout all other languages, but other languages might offer more flexibility around static members or object and as a result this might not be even be such an issue for those languages.

I can't answer all of your questions, but here's what I can say. (I'm no Sentry employee, but I contributed a lot in the past to the SDKs, so I know a lot of the internals of it)

Is there any documentation around testing Sentry for Dart/Flutter?

Speaking from experience of using error monitoring tools, there's no point of testing it. The reporting for global error handlers is already tested by the tool itself, and for other cases I usually used a logger and the monitoring integration with it.

With the current approach, what happens when you would like to have two Sentry instances (with different options) that you would like to report information to? How would you report captured events for one and not the other?

That's an unsupported use case. Due to how Sentry (or rather how error handling in general) works, there should only be one error monitoring tool active at a time. Especially on the native side you could end up not reporting errors at all if you initialize Sentry and another tool.

What happens if a package initialised Sentry to collect information, the application also initialises another Sentry to collect another information, and there is another package that would like to rely on the Sentry that got initialised in the application (not the one from the first package)? What would the static Sentry members (in the latter package) rely on, would it work as expected?

That's an unsupported use case. I suppose using it like that would result in some data loss, but it would report some data.

I'm also curious to know what is the motivation for making everything static and global rather than letting the user have an actual instance?

Well, errors are global and not all global error handlers support adding multiple listeners. With Flutter, this is especially tricky since it needs to work seamless across Flutter and the native platforms. So it's somewhat a result of how error handlers are working, in addition to design choices of the devs of Sentry.

Thanks @ueman for the reply! I'm going to wait for a Sentry employee to get back.

Regarding testing, I do think it is a good practice to assert that the right flow-tracking or error is being sent to Sentry, and hence the question. From the testing documentation for Sentry on Node:

Learn how to assert that the right flow-tracking or error is being sent to Sentry, but without really sending it to the Sentry server

Thanks @alestiago for the questions and @ueman for chiming in - much appreciated! I don't have a lot to add to the above - the scenarios described are unsupported use cases, and we currently don't have plans to provide tooling for 3p testing of the SDK.

Hi!

Just want to quickly add that the Sentry Testkit for JS is community driven so we don't explicitly own it at Sentry.

That might not be exactly what you're looking for but we have support for spotlight with the Flutter SDK which can help you quickly validate Sentry events.

getsentry / sentry-dart