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

Improvement: TorrentSession.kt #6

Closed
z1zong opened this issue May 16, 2019 · 5 comments
Closed

Improvement: TorrentSession.kt #6

z1zong opened this issue May 16, 2019 · 5 comments

Comments

@z1zong
Copy link

z1zong commented May 16, 2019

First, It's a very great SDK and easy to use.
However, I notice some fatal errors in our app's firebase log related to
line 303 to line 326:

private fun downloadUsingNetworkUri(
            downloadLocation: File
            , torrentUrl: URL
    ) {
        val connection = (torrentUrl.openConnection() as HttpURLConnection).apply {
            requestMethod = "GET"
            instanceFollowRedirects = true
        }

        connection.connect()

        if (connection.responseCode != 200) {
            Log.w(Tag, "Unexpected response code returned from server: ${connection.responseCode}")
        }

        val data = connection
                .inputStream
                .readBytes()

        sessionManager.download(
                TorrentInfo.bdecode(data)
                , downloadLocation
        )
    }

I'm guessing that when hostname is not available or somehow url not correct, this method will cause uncaught exceptions: net.ConnectException - Failed to connect to ${some url}. Is it better to put a tray-catch block to catch these exceptions?

@masterwok
Copy link
Owner

Thanks, I'm glad you are finding it useful.

If I were you, I'd try/catch the portion of your application code that results in this function being executed and crashing down the line. This way your application can catch the exception and let the user know they provided an invalid path.

@z1zong
Copy link
Author

z1zong commented May 16, 2019

Sorry, I did state the issue clearly.
this function is called by your SDK after I execute:

 AsyncTask.execute(new Runnable() {
                @Override
                public void run() {
                        torrentSession.start(context, url);
                }
            });

some users cannot connect to our host due to internet censorship, and this method (torrentSession.start(context, url)) cause a crash instead of throwing exceptions in callback listener.
I'm just suggesting that it will be better and an improvement that any errors related to url should be put in TorrentSessionListener.class

@Override
        public void onTorrentError(@NotNull TorrentHandle torrentHandle, @NotNull TorrentSessionStatus torrentSessionStatus) {
            
        }

Thanks for your reply. :)

@masterwok
Copy link
Owner

Thanks for the suggestion. Unfortunately, we can't use onTorrentError(...) callback as there is no TorrentHandle or TorrentSessionStatus instances available at this time. Even if we could, these callbacks are really just bubbling up events from the underlying jlibtorrent implementation so I wouldn't want to overload them.

Would the following work for your use case?

 AsyncTask.execute(new Runnable() {
    @Override
    public void run() {
        try {
            torrentSession.start(context, url);
        catch(ex: Exception) {
            // Take action to notify the user something went wrong
        }
    }
});

@z1zong
Copy link
Author

z1zong commented May 16, 2019

Thanks for your reply.
I will try this way, or find other ways.

@masterwok
Copy link
Owner

No problem, good luck!

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

No branches or pull requests

2 participants