NewStory: Improve the error-handling and reporting

[GoogleCodeExporter]

All system errors and exceptions must be captured and handled by the app. Error 
messages that affect the User need to be displayed to the user where the user 
is able to:
   * understand what happened and whether they can address the problem
   * where practical decide what action to take
   * provide an option for the user to send information about the problem to the project team

When the project team receives information about problems we are better armed 
to try and address problems or limitations in the app. Where problems are 
related to external content or services e.g. to corrupt books or books in 
formats not supported by the libraries we are using in the app we can seek ways 
to address these problems longer term.

Messages to users should be in their selected human language e.g. English, 
French, German, etc. where practical. Otherwise the language will be English.

Original issue reported on code.google.com by julianharty on 19 Jun 2013 at 6:28

[GoogleCodeExporter]

Original comment by julianharty on 19 Jun 2013 at 6:29

[GoogleCodeExporter]

"* provide an option for the user to send information about the problem to the 
project team". I totally agree this item. To be more precise, I'd like to know 
which information that need to be sent to development team. I saw in many apps 
(eg Chrome on Android in the attachment) there are lot of information inside. I 
think we should concentrate to the essential information only. How do you think?

Original comment by tranquangminhtan@gmail.com on 25 Jun 2013 at 8:53

Attachments:

• chrome_android.png

[GoogleCodeExporter]

Let's discuss the balance between the information that would be useful and 
minimising the transmission of sensitive or unnecessary data.

Original comment by julianharty on 25 Jun 2013 at 8:22

[GoogleCodeExporter]

Here is the link to the BugSense documentation for integration of their API 
https://www.bugsense.com/docs/android

For now let's keep the API key private. I have asked BugSense support for their 
recommendations about the sensitivity of making the key visible on the internet 
(which our code base is).

Julian

Original comment by julianharty on 15 Jul 2013 at 3:18

• Changed state: Started

[GoogleCodeExporter]

Here's a good example of a user-centered error message in GMail:

"Something's not right.
We're having trouble contacting our servers. We're going to keep trying."

Currently our code swallows exceptions in many places instead of handling them 
appropriately (e.g. retry, do something else, ask the user, cancel the 
operation, etc). Let's focus on improving the way our code handles errors. 

Original comment by julianharty on 14 Aug 2013 at 8:12

• Added labels: Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

The version of the app released at the end of Sprint 4 sometimes fails with a 
NPE (NullPointerException) when the Download Books Activity launches 
(DaisyBookDownloadBooks). The exception is not caught by our code or by 
bugsense's global exception handler. Therefore we have more work to do on this 
story to address these aspects. I'll help investigate the problem and share 
some of my thoughts and ideas online so we can collectively learn and improve 
the code.

Original comment by julianharty on 21 Aug 2013 at 8:06

[GoogleCodeExporter]

For instance, the most recent BugSense errors are related to the way the logic 
in the app has been structured e.g. 
https://www.bugsense.com/dashboard/project/cdc7bdaf/errors/334918321 (access 
limited to the project team)

Here's my take on what's happening
The error being reported is in Navigator.java - within daisy-engine
So my first thought was - where's the bug in daisy-engine?
- what's causing the code to fail? especially when the stack trace is related 
to the UI code. Then I noticed several related weaknesses in the current code.

They're related to the flow of control when there's a problem opening the daisy 
book.
The user sees a reported problem, but the code then continues to try and open 
the book, however the 'book' object is null therefore Navigator.java finally 
raises an exception which causes flow of control to change.

Here's the worst offender
     private void openBook() {
        if (isFormat202) {
            openBook202();
        } else {
            openBook30();
        }
        AndroidAudioPlayer androidAudioPlayer = new AndroidAudioPlayer(mBookContext);
        androidAudioPlayer.addCallbackListener(audioCallbackListener);
        mAudioPlayer = new AudioPlayerController(androidAudioPlayer);
        mPlayer = androidAudioPlayer.getCurrentPlayer();
        // get all navigator of book to push to table of contents.
        mNavigatorOfTableContents = new Navigator(mBook);
        mNavigator = mNavigatorOfTableContents;
        if (!isFormat202) {
            Navigator temp = new Navigator(mBook);
            listId = new ArrayList<String>();
            while (temp.hasNext()) {
                Section n = (Section) temp.next();
                listId.add(splitHref(n.getHref())[1]);
            }
        }
    }
When an exception is raised in openBook202() and I assume similarly if one is 
raised in openBook30() then flow of control should change e.g. to abandon 
trying to open and read this book.
Currently the code tries to continue regardless so the subsequent code finally 
crashes (well raises an exception)

Unit tests are good, and can help with some of the problems. however in this 
case I think the main issue is we need to think through how errors should be 
detected, reported and handled; and how these errors affect the flow of control.

In essence, a major challenge for us is to design the methods better. 'Better' 
is a vague term, and I'm struggling to explain succinctly what sorts of 
improvements are appropriate. 

We can try applying TDD to some of this code (especially when it's non-UI 
related) to help design better methods by considering, explicitly and early, 
how we want the code to behave in various conditions (or circumstances).

Whenever there's code like:
if (isFormat202) {
            openBook202();
        } else {
            openBook30();
        }

we're not using OO, instead we're writing conditional logic that complicates 
the codebase. 

http://stackoverflow.com/questions/17015946/which-exception-to-throw-for-invalid
-input-which-is-valid-from-client-perspectiv

Original comment by julianharty on 12 Nov 2013 at 12:50