nextcloud/android-library

NPE: RemoteOperationResult - StatusLine.getStatusCode()

grote opened this issue · 0 comments

grote commented

When making heavy use of Nextcloud's DocumentsProvider via Seedvault, I sometimes get crashes like these:

This is with Nextcloud 3.12.1 which seems to be using 4b90880 of this library.

    java.lang.NullPointerException: Attempt to invoke virtual method 'int org.apache.commons.httpclient.StatusLine.getStatusCode()' on a null object reference
        at org.apache.commons.httpclient.HttpMethodBase.getStatusCode(HttpMethodBase.java:570)
        at com.owncloud.android.lib.common.operations.RemoteOperationResult.<init>(RemoteOperationResult.java:328)
        at com.owncloud.android.lib.resources.files.DownloadFileRemoteOperation.run(DownloadFileRemoteOperation.java:85)
        at com.owncloud.android.lib.common.operations.RemoteOperation.execute(RemoteOperation.java:181)
        at com.owncloud.android.operations.DownloadFileOperation.run(DownloadFileOperation.java:160)
        at com.owncloud.android.lib.common.operations.RemoteOperation.execute(RemoteOperation.java:181)
        at com.owncloud.android.files.services.FileDownloader.downloadFile(FileDownloader.java:471)
        at com.owncloud.android.files.services.FileDownloader.access$500(FileDownloader.java:81)
        at com.owncloud.android.files.services.FileDownloader$ServiceHandler.handleMessage(FileDownloader.java:422)

It seems like DownloadFileRemoteOperation#run() is calling DownloadFileRemoteOperation#downloadFile() which swallows all exceptions (not sure if logging works, I didn't see any messages). Then run() is working on the potentially bogus result and in this case crashes. I've often seen a status of -1 returned in the log without crashes, but that probably also means that there was a problem with the HTTP request.

This crash is especially bad, because the DocumentsProvider consumer is hanging waiting for a response from Nextcloud which never comes.

So if DownloadFileRemoteOperation#downloadFile() is only ever called by run() which catches all exceptions already, then I suggest to remove the catching from downloadFile(), so that they appear in run() and this doesn't try to proceed with an undefined result, but errors out properly.