Tuesday, March 30, 2010

How to improve your application : a crash reporter to improve stability ( slight return )

Hi all,

For quite some time, I wanted to return on my crash reporter, and on this article here.

Actually, there are two things that I changed for my crash reporter :
* First, I made it more robust, fixing a bug (that I can't understand, by the way) in it.
* Then, I extended the crash reporter so that it can be even more useful to help me ( and you ? ) in the bug hunt...

1) Making the crash reporter more robust :

At the launch of my crash reporter, I was gathering informations on the application environment, to give me more information on the system the bug was happening on.
The code I was using was this one :
void RecoltInformations( Context context )
{
PackageManager pm = context.getPackageManager();
        try
        {
         PackageInfo pi;
            // Version
            pi = pm.getPackageInfo(context.getPackageName(), 0);
            VersionName = pi.versionName;
            // Package name
            PackageName = pi.packageName;
            // Files dir for storing the stack traces
            FilePath = context.getFilesDir().getAbsolutePath();
            // Device model
            PhoneModel = android.os.Build.MODEL;
            // Android version
            AndroidVersion = android.os.Build.VERSION.RELEASE;
        
            Board = android.os.Build.BOARD;
            Brand  = android.os.Build.BRAND;
            //CPU_ABI = android.os.Build.;
            Device  = android.os.Build.DEVICE;
            Display = android.os.Build.DISPLAY;
            FingerPrint = android.os.Build.FINGERPRINT;
         Host = android.os.Build.HOST;
         ID = android.os.Build.ID;
         //Manufacturer = android.os.Build.;
         Model = android.os.Build.MODEL;
         Product = android.os.Build.PRODUCT;
         Tags = android.os.Build.TAGS;
         Time = android.os.Build.TIME;
         Type = android.os.Build.TYPE;
         User = android.os.Build.USER;
        
        }
        catch (NameNotFoundException e)
        {
                e.printStackTrace();
        }
}
And this code was crashing on some phones...
I still can't understand where this code can crash ( if you have some ideas, I would happily hear them ! ), but it is actually quite easy to fix :
* Catching all the exceptions, and incorporating the getPackageManagerPart in the try part so that its eventual failure would be caught.
* Not calling this function at the start of the application, but only when it really is needed : when a bug occcurs. There was really no need to call it before.

With this sole improvement, I could prevent the crash reporter to crash ( having a debug tool that creates bug is a _bad_ thing ), but still, I was receiving bugs with this info :

Informations :
==============
Version : null
Package : null
FilePath : /data/data/com.alocaly.LetterGame/files
Phone Modelnull
Android Version : null
Board : null
Brand : null
Device : null
Display : null
Finger Print : null
Host : null
ID : null
Model : null
Product : null
Tags : null
Time : 0
Type : null
User : null
Total Internal memory : 274464768
Available Internal memory : 209596416

It means, first that this code is no more crashing and then that the code that was crashing was in the very first lines of the function. So thrown by GetPackageManager, GetPackageName or GetPackageInfo. 
But, from the Android documentation, only the NameNotFoundException could be launched by these functions... And it was handled by my previous code !!
Conclusion : I still don't understand what was happening !


2) Extending the crash reporter.

Taking some time to think about this crash reporter, we could wonder what is the primary goal of this tool. The real goal of this tool is to give us more information on the bugs that occured so that we can understand remotely what was really happening.
The callstack is a very valuable piece of information to get, the environment can give you some clues, but this is just not enough. Trying to understand where a bug lay with so few information is very tricky. 
But we can't add more informations, without doing something really specific for one application...
...
And that was the good idea : we obviously need some information specific for the application !!!
The application developer knows what kind of information would help him understand the bugs !

For instance, in my game 'Word Prospector', I finally had some crash in the function that checks whether or not a word is correct. Knowing what this strange-crash-creating-word is would be very valuable information.

So I decided to add in my crash reporter a way to add any informations. I just added a hash map with some Key/Values strings, add some functions to populate the hash map, and display this hash map in the report the crash reporter is creating when a bug occurs.

This was a really good idea ! I could find that this word was starting with a '?' : the user could enter some letters while quitting the pause state, where all the letters are replaced by some question marks...


So now, when there is a suspicious and hard to understand crash, I can had a lot of value to check, release a new version, and wait in front of my mail box !

Here is the new version of the code.
I still init it from the application object, so I initialize it for the whole game with just one call.


public class ErrorReporter implements Thread.UncaughtExceptionHandler
{
String VersionName;
String PackageName;
String FilePath;
String PhoneModel;
String AndroidVersion;
String Board;
String Brand;
String Device;
String Display;
String FingerPrint;
String Host;
String ID;
String Manufacturer;
String Model;
String Product;
String Tags;
long Time;
String Type;
String User;
HashMap CustomParameters = new HashMap< String, String>();

private Thread.UncaughtExceptionHandler PreviousHandler;
private static ErrorReporter S_mInstance;
private Context CurContext;

public void AddCustomData( String Key, String Value )
{
CustomParameters.put( Key, Value );
}

private String CreateCustomInfoString()
{
String CustomInfo = "";
Iterator iterator = CustomParameters.keySet().iterator();
while( iterator.hasNext() )
{
String CurrentKey = iterator.next();
String CurrentVal = CustomParameters.get( CurrentKey );
CustomInfo += CurrentKey + " = " + CurrentVal + "\n";
}
return CustomInfo;
}

static ErrorReporter getInstance()
{
if ( S_mInstance == null )
S_mInstance = new ErrorReporter();
return S_mInstance;
}

public void Init( Context context )
{
PreviousHandler = Thread.getDefaultUncaughtExceptionHandler();
Thread.setDefaultUncaughtExceptionHandler( this );
CurContext = context;
}

public long getAvailableInternalMemorySize() {
        File path = Environment.getDataDirectory();
        StatFs stat = new StatFs(path.getPath());
        long blockSize = stat.getBlockSize();
        long availableBlocks = stat.getAvailableBlocks();
        return availableBlocks * blockSize;
    }
    
    public long getTotalInternalMemorySize() {
        File path = Environment.getDataDirectory();
        StatFs stat = new StatFs(path.getPath());
        long blockSize = stat.getBlockSize();
        long totalBlocks = stat.getBlockCount();
        return totalBlocks * blockSize;
    }

void RecoltInformations( Context context )
{
        try
        {
     PackageManager pm = context.getPackageManager();
         PackageInfo pi;
            // Version
            pi = pm.getPackageInfo(context.getPackageName(), 0);
            VersionName = pi.versionName;
            // Package name
            PackageName = pi.packageName;
            // Device model
            PhoneModel = android.os.Build.MODEL;
            // Android version
            AndroidVersion = android.os.Build.VERSION.RELEASE;
          
            Board = android.os.Build.BOARD;
            Brand  = android.os.Build.BRAND;
            Device  = android.os.Build.DEVICE;
            Display = android.os.Build.DISPLAY;
            FingerPrint = android.os.Build.FINGERPRINT;
         Host = android.os.Build.HOST;
         ID = android.os.Build.ID;
         Model = android.os.Build.MODEL;
         Product = android.os.Build.PRODUCT;
         Tags = android.os.Build.TAGS;
         Time = android.os.Build.TIME;
         Type = android.os.Build.TYPE;
         User = android.os.Build.USER;
          
        }
        catch( Exception e )
        {
         e.printStackTrace();
        }
}

public String CreateInformationString()
{
RecoltInformations( CurContext );

String ReturnVal = "";
ReturnVal += "Version : " + VersionName;
ReturnVal += "\n";
ReturnVal += "Package : " + PackageName;
ReturnVal += "\n";
ReturnVal += "FilePath : " + FilePath;
ReturnVal += "\n";
ReturnVal += "Phone Model" + PhoneModel;
ReturnVal += "\n";
ReturnVal += "Android Version : " + AndroidVersion;
ReturnVal += "\n";
ReturnVal += "Board : " + Board;
ReturnVal += "\n";
ReturnVal += "Brand : " + Brand;
ReturnVal += "\n";
ReturnVal += "Device : " + Device;
ReturnVal += "\n";
ReturnVal += "Display : " + Display;
ReturnVal += "\n";
ReturnVal += "Finger Print : " + FingerPrint;
ReturnVal += "\n";
ReturnVal += "Host : " + Host;
ReturnVal += "\n";
ReturnVal += "ID : " + ID;
ReturnVal += "\n";
ReturnVal += "Model : " + Model;
ReturnVal += "\n";
ReturnVal += "Product : " + Product;
ReturnVal += "\n";
ReturnVal += "Tags : " + Tags;
ReturnVal += "\n";
ReturnVal += "Time : " + Time;
ReturnVal += "\n";
ReturnVal += "Type : " + Type;
ReturnVal += "\n";
ReturnVal += "User : " + User;
ReturnVal += "\n";
ReturnVal += "Total Internal memory : " + getTotalInternalMemorySize();
ReturnVal += "\n";
ReturnVal += "Available Internal memory : " + getAvailableInternalMemorySize();
ReturnVal += "\n";

return ReturnVal;
}

public void uncaughtException(Thread t, Throwable e)
{
String Report = "";
Date CurDate = new Date();
Report += "Error Report collected on : " + CurDate.toString();
Report += "\n";
Report += "\n";
Report += "Informations :";
Report += "\n";
Report += "==============";
Report += "\n";
Report += "\n";
Report += CreateInformationString();

Report += "Custom Informations :\n";
Report += "=====================\n";
Report += CreateCustomInfoString();

Report += "\n\n";
Report += "Stack : \n";
Report += "======= \n";
final Writer result = new StringWriter();
final PrintWriter printWriter = new PrintWriter(result);
e.printStackTrace(printWriter);
String stacktrace = result.toString();
Report += stacktrace;

Report += "\n";
Report += "Cause : \n";
Report += "======= \n";

// If the exception was thrown in a background thread inside
// AsyncTask, then the actual exception can be found with getCause
Throwable cause = e.getCause();
while (cause != null)
{
cause.printStackTrace( printWriter );
Report += result.toString();
cause = cause.getCause();
}
printWriter.close();
Report += "****  End of current Report ***";
SaveAsFile(Report);
//SendErrorMail( Report );
PreviousHandler.uncaughtException(t, e);
}

private void SendErrorMail( Context _context, String ErrorContent )
{
Intent sendIntent = new Intent(Intent.ACTION_SEND);
String subject = _context.getResources().getString( R.string.CrashReport_MailSubject );
String body = _context.getResources().getString( R.string.CrashReport_MailBody ) +
"\n\n"+
ErrorContent+
"\n\n";
sendIntent.putExtra(Intent.EXTRA_EMAIL,
new String[] {"postmaster@alocaly.com"});
sendIntent.putExtra(Intent.EXTRA_TEXT, body);
sendIntent.putExtra(Intent.EXTRA_SUBJECT, subject);
sendIntent.setType("message/rfc822");
_context.startActivity( Intent.createChooser(sendIntent, "Title:") );
}
private void SaveAsFile( String ErrorContent )
{
try
{
Random generator = new Random();
int random = generator.nextInt(99999);
String FileName = "stack-" + random + ".stacktrace";
FileOutputStream trace = CurContext.openFileOutput( FileName, Context.MODE_PRIVATE);
trace.write(ErrorContent.getBytes());
trace.close();
}
catch( Exception e )
{
// ...
}
}

private String[] GetErrorFileList()
{
File dir = new File( FilePath + "/");
        // Try to create the files folder if it doesn't exist
        dir.mkdir();
        // Filter for ".stacktrace" files
        FilenameFilter filter = new FilenameFilter() {
                public boolean accept(File dir, String name) {
                        return name.endsWith(".stacktrace");
                }
        };
        return dir.list(filter);
}
private boolean bIsThereAnyErrorFile()
{
return GetErrorFileList().length > 0;
}
public void CheckErrorAndSendMail(Context _context )
{
try
{

FilePath = _context.getFilesDir().getAbsolutePath();
if ( bIsThereAnyErrorFile() )
{
String WholeErrorText = "";
// on limite à N le nombre d'envois de rapports ( car trop lent )
String[] ErrorFileList = GetErrorFileList();
int curIndex = 0;
final int MaxSendMail = 5;
for ( String curString : ErrorFileList )
{
if ( curIndex++ <= MaxSendMail )
{
WholeErrorText+="New Trace collected :\n";
WholeErrorText+="=====================\n ";
String filePath = FilePath + "/" + curString;
BufferedReader input =  new BufferedReader(new FileReader(filePath));
String line;
while (( line = input.readLine()) != null)
{
WholeErrorText += line + "\n";
}
input.close();
}

// DELETE FILES !!!!
File curFile = new File( FilePath + "/" + curString );
curFile.delete();
}
SendErrorMail( _context , WholeErrorText );
}
}
catch( Exception e )
{
e.printStackTrace();
}
}
}

Enjoy it !!


NOTE : I'm no more using this mail technic. Now I'm using ACRA, and the results are much more interesting !! See here for more info.


Ps : here is a link I found while googling :
http://web-authoring.seadvd.com/whats-google-up-to-with-android/
:)

12 comments:

Psym said...

Could the crash bug have been a nullpointerexception? I've been caught out by null contexts before, in particular in a service before it has finished binding.

Anonymous said...

I'm not sure if my knowledge is limited when it comes to how the compiler takes care:
String a = "blah";
a += "b";
a += "c";

But i believe this code can screw up the memory, since String is immutable and thus creating a big amount of waste.

Why not use a StringBuilder or StringBuffer instead?

Felipe Silveira said...

Very useful mechanism. Consider to get Heap-dumps too. It helps a lot to investigate Out-of-memory exceptions.

Nick said...

Great Stuff - thanks for sharing.

BTW The code is slightly mangled though - the html has eaten the <String>s in various places.

AndroidBlogger said...

@psym:
Sure, but I don't expected - from the documentation - any null at this point !

@anonymous :
You are definitively right ! Actually, I'm not a java guy at all. But from what I've heard, you are completely right.
Now, when you are sending a crash report, speed is no more a concern :)

@Felipe Silveira :
This is very interesting !!
We can get the HeapDump during a crash ? I have to investigate this !
Thanks !

@Nick :
Hum... I'm definitively not a graphic guy, and my layout is far from being good. But I still don't have the issue you're talking about. Can it be a browser specific one ?

Nick said...

Re: Mising <String>s...

For example the line

HashMap CustomParameters = new HashMap< String, String>();

Should read

HashMap<String, String> CustomParameters = new HashMap< String, String>();

I'm guessing that these got eaten by some HTML encoding at some point

Majid said...

Thank you very much!

Just implemented it in my app. This should reduce a lot of user complaints.

Thanks again.

britishturbo said...

Thanks,

Just added this to my app I'm working on... a Twitter App called Tweetissimo... and it's already helped me catch 3 bugs int he last hour!

Cheers!

Nil said...

Hi All,

I am newbie in Programming and Android and this seems to me an excellent mechanism. Can you pls help me how this would trigger. Do I have to create instance of this class to initialize it before.

Thanks;
nil

K. S. said...

http://jyro.blogspot.com/2009/09/crash-report-for-android-app.html
You used about 90% of that solution without crediting the author. Stealing is a sin.

AndroidBlogger said...

Hi K.S.,

By no way, I was stealing the code from this blog. I didn't know about it at all.
I knew the stackoverflow subject, and I was inspired by this subject and this one :http://code.google.com/p/android-remote-stacktrace/
to create the error catching part of the code.

My contributions, that were only from my work, and that I really think added value to the whole process were :
* Adding some specific information (like OS version, phone version, apk version, memory left, ... )
* Adding customisable information for the app ( and this is a huge improvement )
* Adding the mail part : I didn't want to have a web server to set up, so I found the mail solution.
* Using the application to set up the whole solution

At the moment I wrote this, I didn't heard of anybody doing this kind of thing, and I posted my improvement on the stackoverflow thread.
I imagine Jiro had the same idea (using mail instead of a web server ) before me.
It's far from being uncommon that two developers have the same idea at the same time, just because it is a adapted response to a particular issue.

Two other notes :
* I'm not using this system anymore, someone took my code and greatly improves it to create ACRA.
I really need to make a blog entry on this, I really think it is THE ultimate crash reporter !
* I'm glad you bring this blog to my attention, there a few interesting things ( auto login for google for instance )

Peace...

Suriya said...

Thanks for the article. Lot of manufactures tweek the android.os.Build object. Hence accessing field directly should be avoided in the program as it would cause class loading error if the manufacture remove that field or that field is not available on that os. To make the code work on all scenarios I used reflect api to access the Build fields.

StringBuilder bldr = new StringBuilder();
Class build = android.os.Build.class;

for(Field curr: build.getFields())
{
if(curr.getType().equals(String.class))
{
try
{
bldr.append("Build->" + curr.getName() + ":").append("" + curr.get(build)).append("\n");
}
catch(Exception e)
{
Log.d("Suriya", "Build info exception.", e);
}
}
}

This solves Class loading error and also if new field is added on OS crash report would include those fields.