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

[SES-1930] Catch HTTP exceptions from threads #1491

Merged
merged 9 commits into from
May 21, 2024

Conversation

AL-Session
Copy link
Collaborator

@AL-Session AL-Session commented May 20, 2024

Contributor checklist

  • I have tested my contribution on these devices:
  • Virtual Pixel 3a, Android 9 / API 28
  • Virtual Pixel 3a, Android 14 / API 34
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Exceptions thrown from HTTP.kt have been causing crashes - specifically when the device is not connected to the network (HTTPNoNetworkConnection) or if an operation failed due to a timeout (HTTPRequestFailedException). This PR simply catches the exceptions and logs them in the ThreadUtil.queue functions to prevent a crash.

@AL-Session AL-Session requested a review from simophin May 20, 2024 04:39
Copy link

@bemusementpark bemusementpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this catches exceptions thrown from scheduling this, rather than the actual work itself, unless the executor runs immediately on the current thread.

I guess you can just move the catch inside.

@bemusementpark
Copy link

In the stack trace it doesn't go through ThreadUtil#queue, but queue$lambda$0 the anonymous function, which is kind of a clue apparently, cause that's called asynchronously by ThreadPoolExecutor

at org.session.libsignal.utilities.ThreadUtils.queue$lambda$0 (ThreadUtils.kt:18)

…n pool reasons & added a thread limiting mechanism to prevent excessive thread creation (when the queue is full then further tasks are queued)
@AL-Session AL-Session changed the title Ses1930 catch http exceptions [SES-1930] Catch HTTP exceptions from threads May 21, 2024
@AL-Session AL-Session merged commit 658f7de into oxen-io:dev May 21, 2024
1 check was pending
@AL-Session AL-Session deleted the SES1930_CatchHTTPExceptions branch July 22, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants