getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.14k stars 432 forks source link

No way to set NoOpTransportFactory for SentryClient in order to mock only transport part #3555

Open bitsal opened 1 month ago

bitsal commented 1 month ago

Integration

sentry-spring-boot

Java Version

8

Version

7.9.0

Steps to Reproduce

By default Sentry has NoOpTransportFactory set, but it is overridden to AsyncHttpTransportFactory inside SentryClient.

Expected Result

I think it can be flexible and this hardcode should rely on SentryOptions or other parameters that say that an application want to use real transport protocol instead of NoOp one.

Actual Result

No way to have only transport part is mocked for tests needs and Sentry fails in logs

ERROR: Envelope submission failed 
 java.lang.IllegalStateException: Sending the event failed.
java.lang.IllegalStateException: Sending the event failed.
    at io.sentry.transport.AsyncHttpTransport$EnvelopeSender.flush(AsyncHttpTransport.java:336)
    at io.sentry.transport.AsyncHttpTransport$EnvelopeSender.run(AsyncHttpTransport.java:243)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
    at java.util.concurrent.FutureTask.run(FutureTask.java)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: java.net.ConnectException: Connection refused (Connection refused)
    at java.net.PlainSocketImpl.socketConnect(Native Method)
romtsn commented 1 month ago

hm, can't you just create your own no-op factory in tests and set it via options? Then it will not get overwritten by the client

adinauer commented 1 month ago

@bitsal what you're asking for should already be in the SDK if you use setTransportFactory on SentryOptions as Roman suggested.

bitsal commented 1 month ago

NoOp transport factory is the default value, see SentryOptions.

private @NotNull ITransportFactory transportFactory = NoOpTransportFactory.getInstance();

Even I set it, it will be overridden in SentryClient.

SentryClient(final @NotNull SentryOptions options) {
    this.options = Objects.requireNonNull(options, "SentryOptions is required.");
    this.enabled = true;

    ITransportFactory transportFactory = options.getTransportFactory();
    if (transportFactory instanceof NoOpTransportFactory) {
      transportFactory = new AsyncHttpTransportFactory();
      options.setTransportFactory(transportFactory);
    }
...
adinauer commented 1 month ago

@bitsal yes, but you can create your own implementation of NoOpTransportFactory that implements ITransportFactory but isn't detected by the instanceof check.

bitsal commented 1 month ago

@adinauer as a workaround of course I can do this and many other things, but I want to have it in the SDK. and second better to set AsyncHttpTransportFactory as a default factory and remove this if-statement in SentryClient, it will help to use your NoOpTransportFactory w/o any custom classes

adinauer commented 1 month ago

I guess that part of the code has just grown this way. I think we can change this as you suggested but I can't give an ETA for it. Please use the workaround for now. Thanks for the suggestion.

bitsal commented 1 month ago

Thank you! I will. Can I contribute to speed it up?

adinauer commented 1 month ago

You're very welcome to contribute, please target our 8.x.x branch as we're in the middle of moving to a new major version.