sentry-dart icon indicating copy to clipboard operation
sentry-dart copied to clipboard

Migrate Client to HttpClient to offer the proxy setup

Open onewilk opened this issue 4 years ago • 9 comments
trafficstars

In some case, developers need to config httpClient proxy settings to request the public network cause company's network security requirement.

So here is the problem:

sentry_options.dart

set httpClient(Client httpClient) => _httpClient = httpClient ?? _httpClient;

Client is an abstract class from the http lib, and it doesn't offer the proxy setup yet.

Please migrate Client to HttpClient from dart:io.

then developers could set proxy just like this

httpClient.findProxy = (url) => "PROXY xxxx:xxxx";

or just use project's global singleton HttpClient object directly.

Sorry for my poor english ~

onewilk avatar Mar 30 '21 07:03 onewilk

@onewilk thanks for reporting, we'll discuss this internally and see how much effort :)

marandaneto avatar Mar 30 '21 08:03 marandaneto

a downside of it would be that this lib does not support Browser Apps, so we'd need to have a specific Client for Web

marandaneto avatar Apr 07 '21 09:04 marandaneto

Because of the limitation on dart2js/flutter web, I'm not sure this is the way to go. We could add an abstraction to our use of http, and use our current implementation in the box, but allow users to swap it out as the OP requested.

bruno-garcia avatar May 05 '21 13:05 bruno-garcia

You can set SentryOptions.client to use a IOClient. The IOClient constructor allows to use a dart:io HttpClient.

That should fix the original issue.

Remarks:

  • It might be needed to also change the transport to the http transport.
  • You'll loose some benefits of the native integration
  • Configuration of the native SDK is tracked at #265 if you want to configure those too

As the issue should be resolved now, I think this can be closed.

We should probably document this somewhere.

What do you think, @marandaneto ?

ueman avatar Jun 25 '21 19:06 ueman

@ueman not really, the idea is that there's an easy way to set up a proxy via options and not replacing the whole transport layer. the current http lib does not allow that, but HttpClient does, but it does not work for Web, so we'd need to think this thoroughly

marandaneto avatar Jun 28 '21 07:06 marandaneto

You can't do a proxy setup in JS though. That's why it isn't available in the HTTP package by default. Using the IOClient from the HTTP package does what the original issue is about. You can do this already and it works for the Dart SDK, Windows and Linux. For platforms with a native integration, I would say this should be done with #265

Example :

import 'dart:io';
import 'package:http/io_client.dart';
import 'package:sentry/sentry.dart';

Sentry.init(
  (options) {
    final httpClient = HttpClient(); // from dart:io
    httpClient.findProxy = (url) => "PROXY xxxx:xxxx";
    final ioClient = IOClient(httpClient); // from package:http
    options.client = ioClient;
  }
)

ueman avatar Jun 28 '21 09:06 ueman

@ueman if that works, sounds good, but instead of people doing such code snippet, we could do something like:

https://github.com/getsentry/sentry-java/blob/main/sentry/src/main/java/io/sentry/SentryOptions.java#L134-L138

so they configure host, port, user, pass and we do the rest, this should be done then only by non-web env. and also, configure the native platforms for flutter (pass down the configuration when its available via message channel) as they are the ones sending the events.

if the issue is only about the proxy configuration, that's good.

the other question is, do we want to get rid of http package and use only HttpClient which is builtin on the Dart SDK?

marandaneto avatar Jun 28 '21 09:06 marandaneto

the other question is, do we want to get rid of http package and use only HttpClient which is builtin on the Dart SDK?

The built-in client is only available on dart:io. We would have to use XHR (ort he fetch api) on web. So basically we would need to write something like the HTTP package ourself. I would say it's not worth it.

ueman avatar Jun 28 '21 09:06 ueman

sounds good, lets just make it easier to set up the proxy then (internally using the IOClient) as suggested, but let's keep the Client lib, this is not a breaking change anymore then but rather a feat, will remove the 6.0.0 milestone.

marandaneto avatar Jun 28 '21 10:06 marandaneto