enviroCar / enviroCar-app

enviroCar Android Application
https://envirocar.org
GNU General Public License v3.0
88 stars 154 forks source link

Improving user feedback for track uploading errors #870

Open SebaDro opened 2 years ago

SebaDro commented 2 years ago

Description If an error occurs during upload of all local tracks, the user does not get a proper feedback about the reason for the upload failure. The error message just shows "An error occurred during the upload process.". A more meaningful message text would be necessary to assess if the failure is effected by a misuse of the app, an app error or a server failure. In addition, the error is not logged, thus it does not appear within the log report.

Branches master, develop

How to reproduce Record a small track with and finish recording. Go to the LocalTrackFragment, turn off internet and click the button for uploading all tracks => the app shows "An error occurred during the upload process."

How to fix Adjust error handling here: https://github.com/enviroCar/enviroCar-app/blob/98403d7bc73e246bdae761e515c78c3e688a0dfa/org.envirocar.app/src/org/envirocar/app/views/tracklist/TrackListLocalCardFragment.java#L530-L536

Add error logging and enhance the error handling. Maybe, distinguishing the error type such as for single track upload errors would be useful: https://github.com/enviroCar/enviroCar-app/blob/98403d7bc73e246bdae761e515c78c3e688a0dfa/org.envirocar.app/src/org/envirocar/app/views/tracklist/TrackListLocalCardFragment.java#L390-L418

cdhiraj40 commented 2 years ago

@SebaDro I think its the same issue as #857, but it's better to make the issue alone here from other small fixes like #857

SebaDro commented 2 years ago

Hey @cdhiraj40. Yes, you're right. In parts, the PR adresses this issue. However, it only focusses on the internet connection problem. A proper handling of other error types is still pending. So, I think it's ok to leave both issues open.

cdhiraj40 commented 2 years ago

Hey @cdhiraj40. Yes, you're right. In parts, the PR adresses this issue. However, it only focusses on the internet connection problem. A proper handling of other error types is still pending. So, I think it's ok to leave both issues open.

yes I totally agree, I will look into the issue and see why it's not working, thanks :)

cdhiraj40 commented 2 years ago

@SebaDro I think its happening cause here

public void onError(Throwable e) {

its throwing NullPointerException that's why this never gets true

            if (e instanceof TrackUploadException) {

the reason this keeps on getting printed

showSnackbar(R.string.track_list_upload_track_general_error);

detailed version :

java.lang.NullPointerException cannot be cast to org.envirocar.core.exception.TrackUploadException

and the NullPointerException is coming because (not sure)

 E/enviroCar: [org.envirocar.app.handler.TrackUploadHandler] Attempt to invoke virtual method 'io.reactivex.Observable io.reactivex.Observable.map(io.reactivex.functions.Function)' on a null object reference:

in the file src/org/envirocar/app/handler/agreement/AgreementManager.java lines around AgreementManager.java:125

SebaDro commented 2 years ago

Yes, I noticed the NullPointerException, too. If you follow the stacktrace, you will find, that the exception occurs inside the AgreementManager when it comes to check the terms of use: https://github.com/enviroCar/enviroCar-app/blob/98403d7bc73e246bdae761e515c78c3e688a0dfa/org.envirocar.app/src/org/envirocar/app/handler/agreement/AgreementManager.java#L119-L139

It seems, there are some implementions missing for the CacheTermsOfUseDAO.

However, I don't think there is a necessity to check the terms of use, if there is no internet. I'd prefer a "fail fast" solution e.g. checking internet connection and showing an error dialog without trying to upload a track.

cdhiraj40 commented 2 years ago

Yes, I noticed the NullPointerException, too. If you follow the stacktrace, you will find, that the exception occurs inside the AgreementManager when it comes to check the terms of use:

https://github.com/enviroCar/enviroCar-app/blob/98403d7bc73e246bdae761e515c78c3e688a0dfa/org.envirocar.app/src/org/envirocar/app/handler/agreement/AgreementManager.java#L119-L139

It seems, there are some implementions missing for the CacheTermsOfUseDAO.

However, I don't think there is a necessity to check the terms of use, if there is no internet. I'd prefer a "fail fast" solution e.g. checking internet connection and showing an error dialog without trying to upload a track.

I think I did try that in #857, maybe we can make functions for particular errors and use it? It may decrease these errors coming in future, as we already have some functions made for some issues it will become easier to implement them too

SebaDro commented 2 years ago

Have been fixed in parts with #874. However, the app still does not inform the user about the cause for a failing track upload.

cdhiraj40 commented 2 years ago

@SebaDro sir is this issue from the server side? if not then we can try handling one by one and check why its not working properly

SebaDro commented 2 years ago

I'm not sure if the server responds with a meaningful error message for each track. First step would be to debug the app and analyze the exceptions.