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

Unable to use no_default_http_client #2453

Closed
Masadow opened this issue Nov 27, 2024 · 11 comments
Closed

Unable to use no_default_http_client #2453

Masadow opened this issue Nov 27, 2024 · 11 comments

Comments

@Masadow
Copy link

Masadow commented Nov 27, 2024

Platform

Dart Web

Obfuscation

Enabled

Debug Info

Enabled

Doctor

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.24.3, on macOS 14.1.1 23B81 darwin-x64, locale en-FR)
[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 15.1)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2022.3)
[✓] VS Code (version 1.95.1)
[✓] Connected device (2 available)
[✓] Network resources

• No issues found!

Version

8.10.1

Steps to Reproduce

Set up a custom HttpClient

await SentryFlutter.init(
      (options) {
        options.httpClient = customHttpClient();
        ... other options settings
      },
    );

Then add the parameter --dart-define=no_default_http_client=true to flutter build to ensure no default clients are in use

Expected Result

App starting properly

Actual Result

App is raising an expection because custom http client is not passed to package_info_plus which then use the default HTTP client

Are you willing to submit a PR?

Yes

@buenaflor
Copy link
Contributor

hey thanks for raising this issue, we're having a look

@buenaflor buenaflor moved this from Needs Discussion to Needs Investigation in Mobile & Cross Platform SDK Nov 27, 2024
@martinhaintz
Copy link
Contributor

Hi @Masadow,

I tried reproducing your exception, but wasn't lucky.

Here is what I tried:

I used our sample App.

Then I created a customHttpClient

import 'dart:convert';
import 'dart:typed_data';

import 'package:http/http.dart';

class customHttpClient implements Client {
  @override
  void close() {}

  @override
  Future<Response> delete(Uri url,
      {Map<String, String>? headers, Object? body, Encoding? encoding}) async {
    return Response("foo", 200);
  }

  @override
  Future<Response> get(Uri url, {Map<String, String>? headers}) async {
    return Response("foo", 200);
  }

  @override
  Future<Response> head(Uri url, {Map<String, String>? headers}) async {
    return Response("foo", 200);
  }

  @override
  Future<Response> patch(Uri url,
      {Map<String, String>? headers, Object? body, Encoding? encoding}) async {
    return Response("foo", 200);
  }

  @override
  Future<Response> post(Uri url,
      {Map<String, String>? headers, Object? body, Encoding? encoding}) async {
    return Response("foo", 200);
  }

  @override
  Future<Response> put(Uri url,
      {Map<String, String>? headers, Object? body, Encoding? encoding}) async {
    return Response("foo", 200);
  }

  @override
  Future<String> read(Uri url, {Map<String, String>? headers}) async {
    return "foo";
  }

  @override
  Future<Uint8List> readBytes(Uri url, {Map<String, String>? headers}) async {
    return Uint8List(0);
  }

  @override
  Future<StreamedResponse> send(BaseRequest request) async {
    return StreamedResponse(Stream.empty(), 200);
  }
}

I added it in the main.yaml

Future<void> setupSentry(
  AppRunner appRunner,
  String dsn, {
  bool isIntegrationTest = false,
  BeforeSendCallback? beforeSendCallback,
}) async {
  await SentryFlutter.init(
    (options) {
      options.httpClient = customHttpClient();
      options.dsn = exampleDsn;
      options.tracesSampleRate = 1.0;
      options.profilesSampleRate = 1.0;
...

Build it with fvm flutter build web --release --source-maps --dart-define=no_default_http_client=true

Started the webserver with: python3 -m http.server 8089 and had no exception at app start:
Image

@Masadow can you please be a little bit more specific under which circumstances the error occurs or provide a minimal working project where the error occurs? Thanks.

For completeness, here is my flutter doctor:

fvm flutter doctor -v
[✓] Flutter (Channel stable, 3.24.3, on macOS 15.1.1 24B91 darwin-arm64, locale en-AT)
    • Flutter version 3.24.3 on channel stable at /Users/martin/fvm/versions/3.24.3
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 2663184aa7 (3 months ago), 2024-09-11 16:27:48 -0500
    • Engine revision 36335019a8
    • Dart version 3.5.3
    • DevTools version 2.37.3

[!] Android toolchain - develop for Android devices (Android SDK version 35.0.0)
    • Android SDK at /Users/martin/Library/Android/sdk
    • Platform android-35, build-tools 35.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.11+0-17.0.11b1207.24-11852314)
    ! Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses

[✓] Xcode - develop for iOS and macOS (Xcode 16.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 16B40
    • CocoaPods version 1.16.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2024.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.11+0-17.0.11b1207.24-11852314)

[✓] VS Code (version 1.95.3)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.100.0

[✓] Connected device (3 available)
    • macOS (desktop)                 • macos                 • darwin-arm64   • macOS 15.1.1 24B91 darwin-arm64
    • Mac Designed for iPad (desktop) • mac-designed-for-ipad • darwin         • macOS 15.1.1 24B91 darwin-arm64
    • Chrome (web)                    • chrome                • web-javascript • Google Chrome 131.0.6778.86

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@ueman
Copy link
Collaborator

ueman commented Nov 27, 2024

Maybe you can try to run the following code before initializing Sentry:

final client = createCustomClient();
PackageInfoPlatform.instance = PackageInfoPlusWebPlugin(client);

options.httpClient is the client used for Sentry. It's not intended to be used for package_info_plus.

@Masadow
Copy link
Author

Masadow commented Nov 28, 2024

I forgot to mention that the error occurs when ran in a zonedGuarded

Here's my main :

Future<void> main() async {
  await runZonedGuarded(() async {
    await dotenv.load(fileName: "assets/.env");
    await SentryFlutter.init(
      (options) {
        options.httpClient = httpClient();
        options.dsn = dotenv.get('SENTRY_DSN');
        // Set tracesSampleRate to 1.0 to capture 100% of transactions for performance monitoring.
        // We recommend adjusting this value in production.
        options.tracesSampleRate = 1.0;
        // The sampling rate for profiling is relative to tracesSampleRate
        // Setting to 1.0 will profile 100% of sampled transactions:
        options.profilesSampleRate = 1.0;
        options.environment = dotenv.get('STAGE');
      },
    );
    // ...

Also, the error occurs silently, there's no error in the chrome console, no error on flutter debug console.

However, the app hang on sentry init, no request is made to the network and only way to catch the error is from "Sources" tab in chrome dev tool by ticking both "Pause on exceptions" and from there, you can see an error popping from package_info_plus

I'll try to replicate the issue with more details, I turned the flag off and only tested it on a live server

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 28, 2024
@Masadow
Copy link
Author

Masadow commented Nov 28, 2024

Image

I can confirm I reproduce the issue

Also, I confirm that only sentry uses package_info_plus

Image

Image

As you can see, package info plus can take an optional http client that is never passed the way its made

Honestly, I looked at the source code and have absolutely no idea on how that could be fixed since package_info_plus is registered globally

There's probably an issue you need to raise to them to then be able to fix the issue on your own but I might be wrong, I don't know much at this level of dart code

@Masadow
Copy link
Author

Masadow commented Nov 28, 2024

Maybe you can try to run the following code before initializing Sentry:

final client = createCustomClient();
PackageInfoPlatform.instance = PackageInfoPlusWebPlugin(client);
options.httpClient is the client used for Sentry. It's not intended to be used for package_info_plus.

Looks hacky to me, I don't feel like I should deal with package_info_plus since I don't even import it myself

I also find it weird that it needs any http client for the usage you have with it

I agree that the client passed to sentry is an http client to be used by sentry
Therefore, it might be a good solution to provide options.packageInfoHttpClient too if that fixes the issue ?

I also don't see a ton of usages where you would want a different client to be used by sentry vs package info.

@martinhaintz
Copy link
Contributor

martinhaintz commented Dec 3, 2024

Hello @Masadow

I opened an issue at the plus plugins. In the meantime, you can set options.release and options.dist as a workaround. Setting these two values will prevent calling the PackageInfo plugin in LoadReleaseIntegration.

BR Martin

@martinhaintz
Copy link
Contributor

Hello @Masadow,

As mentioned by @miquelbeltran here, passing the HttpClient into the PackageInfo package is currently not supported. As Sentry uses the PackageInfo package (which fetches the version.json) to get platform infos for the Web platform.

As I mentioned in my previous comment, the code which leads to your error is only executed if options.release == null or options.dist == null. If one of them is null, Sentry will call the PackageInfo and set the options.release or options.dist as these values will be needed for the Sentry Events.

Another way of dealing, would be the approach of @ueman from the previous comment here.

And the third (and probably the most time-consuming way) would be as @miquelbeltran wrote here, to fork the PackageInfo, implement the feature to pass a HttpClient, create a pull request, wait for approval, implement the updated PackageInfo in Sentry, create a pull request, wait for approval and then use it. Or you can just fork the PackageInfo and SentryDart and implement it on your own and use it in your application.

I hope one of the above options fits your application without causing too much overhead.

BR Martin

@martinhaintz martinhaintz moved this from Needs Investigation to Needs More Information in Mobile & Cross Platform SDK Dec 4, 2024
@buenaflor buenaflor moved this to Waiting for: Community in GitHub Issues with 👀 3 Dec 4, 2024
@miquelbeltran
Copy link

And the third (and probably the most time-consuming way) would be as miquelbeltran wrote fluttercommunity/plus_plugins#3381 (comment), to fork the PackageInfo, implement the feature to pass a HttpClient, create a pull request, wait for approval, implement the updated PackageInfo in Sentry...

Hello, if I am not wrong, you can do a dependency overrides with your fork, you don't need to wait for Sentry to incorporate the package.

So you can fork package_info_plus inside your project repo, do the changes you need, and use https://dart.dev/tools/pub/dependencies#dependency-overrides to override the dependency with your custom implementation.

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 4, 2024
@buenaflor
Copy link
Contributor

Hello, if I am not wrong, you can do a dependency overrides with your fork, you don't need to wait for Sentry to incorporate the package.

that would work, yeah

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Dec 6, 2024
@getsantry getsantry bot moved this to Waiting for: Community in GitHub Issues with 👀 3 Dec 10, 2024
@buenaflor buenaflor closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2024
@github-project-automation github-project-automation bot moved this from Needs More Information to Done in Mobile & Cross Platform SDK Dec 13, 2024
@Masadow
Copy link
Author

Masadow commented Dec 13, 2024

thanks for detailed explanation, I'll have a look at that, I'm not a huge fan of forking libraries on my own as I will have to deal with updates manually

However, the solution from @martinhaintz looks enough to me so I'll go with that

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

No branches or pull requests

5 participants