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

How to set SocketTimeout longer #1149

Closed
philos3 opened this issue Nov 21, 2022 · 13 comments · Fixed by #1397
Closed

How to set SocketTimeout longer #1149

philos3 opened this issue Nov 21, 2022 · 13 comments · Fixed by #1397
Assignees
Milestone

Comments

@philos3
Copy link

philos3 commented Nov 21, 2022

Caused by: java.net.SocketTimeoutException: failed to connect to XXXX/XX.XX.130.150 (port 443) from /XX.XX.3.122 (port 41780) after 5000ms

how can I set SocketTimeout longger

@marandaneto
Copy link
Contributor

marandaneto commented Nov 21, 2022

@philos3 are you using sentry or sentry_dart?
If sentry, that's not possible since https://pub.dev/packages/http does not support this configuration.
If sentry_flutter, we'd need to expose these 2 configs on the Dart SDK first.
https://github.com/getsentry/sentry-java/blob/4a9c176e62e67746928d2d4ab455330cdb879c83/sentry/src/main/java/io/sentry/SentryOptions.java#L241
Would you like to submit a PR?

@marandaneto
Copy link
Contributor

I suspect iOS does not have such configurations, it might be needed to be exposed first (NSURLSessionConfiguration).
@philipphofmann or @brustolin to confirm.

@brustolin
Copy link
Collaborator

We will need to expose this property first in Cocoa SDK. But its possible.

@philipphofmann philipphofmann changed the title how to set SocketTimeout longger How to set SocketTimeout longer Nov 24, 2022
@philipphofmann
Copy link
Member

I would like to understand your underlying problem, @philos3. When do you get the error? Why do you think you can solve it by increasing the socket timeout?

@marandaneto
Copy link
Contributor

SocketTimeoutException

Well if you have an unstable connection, maybe 5s (default) isn't enough for a big portion of your customers, it's a known problem worldwide, so it should be at least configurable, obviously, this is remediation.

@philipphofmann
Copy link
Member

Well if you have an unstable connection, maybe 5s (default) isn't enough for a big portion of your customers, it's a known problem worldwide, so it should be at least configurable, obviously, this is remediation.

I think we should come up with a solution that works out of the box. We should guarantee that the SDKs send the events to Sentry under such conditions. I think we shouldn't solve this problem by making it configurable.

@marandaneto
Copy link
Contributor

marandaneto commented Nov 25, 2022

@philipphofmann I'm not sure I follow your comment, we cannot do much, the problem isn't in the SDK, it's in people's connection.
This is the OS trying to connect to the host.

@philos3
Copy link
Author

philos3 commented Nov 28, 2022

5000ms was too short in unstable connection ,and my network is fine. It's just because of a timeout, so i wish I can set the timeout config myself

@philipphofmann
Copy link
Member

the problem isn't in the SDK, it's in people's connection.

Yes, but we could use a longer socket timeout if we get a SocketTimeoutException, or maybe retry.

@philos3
Copy link
Author

philos3 commented Jan 7, 2023

this SocketTimeoutException is cause by the sentry sdk when in unstable connection. Sentry fail to report after 5000ms,and throw SocketTimeoutException(default 5000ms,I can't do anything) ,it is not my business code SocketTimeoutException,so I wish to set sentry SDK SocketTimeout longer ,maybe 7s can avoid receiving a lot of SocketTimeoutException

@marandaneto
Copy link
Contributor

Created getsentry/sentry-cocoa#2658

@philipphofmann
Copy link
Member

The default timeout of NSURLSession is 60 seconds. So I don't think this is an issue on iOS. Can't we increase the default value on Java to solve this issue for all users instead of exposing options to configure? The option has the downside of users having to be aware of it and we need to maintain it.

@marandaneto
Copy link
Contributor

We can add those 2 values on SentryFlutterOptions and add property docs that this is Android only, iOS is 60s by default.

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

Successfully merging a pull request may close this issue.

5 participants