Seriously improve error reporting in DownloadThread.

My old error reporting strategy for DownloadThread was to log the
stack trace for the exception, so we'd know exactly what conditions
caused the StopRequest.  hackbod suggested that we shouldn't log
tracebacks as they clutter the log.  Instead, we should just always
include a little string tag explaining why the request is being
stopped -- this is more concise and more useful to developers.

There are three main changes here to acheive this goal:
* make StopRequest require a short, log-friendly error message upon
  construction, and add such a message to all construction sites
* make a similar change to GenerateSaveFileError, so that the variety
  of errors that originate with Helpers.generateSaveFile() get
  similarly fine-grained and concise error reporting
* make network usable checking code return a distinct error code for
  each distinct negative condition, and add a utility to return a
  log-friendly error message for each such code.

Finally, I cleaned up some of the ways errors/exceptions are handled
in the process.

Change-Id: Ie70cbf3f2960e260e97f8449258e25218d0f900f

4 files changed, 164 insertions, 198 deletions

diff --git a/src/com/android/providers/downloads/DownloadFileInfo.java b/src/com/android/providers/downloads/DownloadFileInfo.java
deleted file mode 100644
index ce423880..00000000
--- a/src/com/android/providers/downloads/DownloadFileInfo.java
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Copyright (C) 2008 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.providers.downloads;
-
-import java.io.FileOutputStream;
-
-/**
- * Stores information about the file in which a download gets saved.
- */
-public class DownloadFileInfo {
-    String mFileName;
-    FileOutputStream mStream;
-    int mStatus;
-
-    public DownloadFileInfo(String fileName, FileOutputStream stream, int status) {
-        mFileName = fileName;
-        mStream = stream;
-        mStatus = status;
-    }
-}
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index b6f478c8..3327e80b 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -175,9 +175,9 @@ public class DownloadInfo {
     public static final int NETWORK_OK = 1;
 
     /**
-     * The network is unusuable for some unspecified reason.
+     * There is no network connectivity.
      */
-    public static final int NETWORK_UNUSABLE_GENERIC = 2;
+    public static final int NETWORK_NO_CONNECTION = 2;
 
     /**
      * The download exceeds the maximum size for this network.
@@ -191,6 +191,16 @@ public class DownloadInfo {
     public static final int NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE = 4;
 
     /**
+     * The current connection is roaming, and the download can't proceed over a roaming connection.
+     */
+    public static final int NETWORK_CANNOT_USE_ROAMING = 5;
+
+    /**
+     * The app requesting the download specific that it can't use the current network connection.
+     */
+    public static final int NETWORK_TYPE_DISALLOWED_BY_REQUESTOR = 6;
+
+    /**
      * For intents used to notify the user that a download exceeds a size threshold, if this extra
      * is true, WiFi is required for this download size; otherwise, it is only recommended.
      */
@@ -340,10 +350,10 @@ public class DownloadInfo {
     public int checkCanUseNetwork() {
         Integer networkType = mSystemFacade.getActiveNetworkType();
         if (networkType == null) {
-            return NETWORK_UNUSABLE_GENERIC;
+            return NETWORK_NO_CONNECTION;
         }
         if (!isRoamingAllowed() && mSystemFacade.isNetworkRoaming()) {
-            return NETWORK_UNUSABLE_GENERIC;
+            return NETWORK_CANNOT_USE_ROAMING;
         }
         return checkIsNetworkTypeAllowed(networkType);
     }
@@ -357,6 +367,32 @@ public class DownloadInfo {
     }
 
     /**
+     * @return a non-localized string appropriate for logging corresponding to one of the
+     * NETWORK_* constants.
+     */
+    public String getLogMessageForNetworkError(int networkError) {
+        switch (networkError) {
+            case NETWORK_RECOMMENDED_UNUSABLE_DUE_TO_SIZE:
+                return "download size exceeds recommended limit for mobile network";
+
+            case NETWORK_UNUSABLE_DUE_TO_SIZE:
+                return "download size exceeds limit for mobile network";
+
+            case NETWORK_NO_CONNECTION:
+                return "no network connection available";
+
+            case NETWORK_CANNOT_USE_ROAMING:
+                return "download cannot use the current network connection because it is roaming";
+
+            case NETWORK_TYPE_DISALLOWED_BY_REQUESTOR:
+                return "download was requested to not use the current network type";
+
+            default:
+                return "unknown error with network connectivity";
+        }
+    }
+
+    /**
      * Check if this download can proceed over the given network type.
      * @param networkType a constant from ConnectivityManager.TYPE_*.
      * @return one of the NETWORK_* constants
@@ -365,7 +401,7 @@ public class DownloadInfo {
         if (mIsPublicApi) {
             int flag = translateNetworkTypeToApiFlag(networkType);
             if ((flag & mAllowedNetworkTypes) == 0) {
-                return NETWORK_UNUSABLE_GENERIC;
+                return NETWORK_TYPE_DISALLOWED_BY_REQUESTOR;
             }
         }
         return checkSizeAllowedForNetwork(networkType);
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index b3847715..2995bfb6 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -26,7 +26,6 @@ import android.os.PowerManager;
 import android.os.Process;
 import android.provider.Downloads;
 import android.provider.DrmStore;
-import android.util.Config;
 import android.util.Log;
 import android.util.Pair;
 
@@ -111,16 +110,21 @@ public class DownloadThread extends Thread {
     /**
      * Raised from methods called by run() to indicate that the current request should be stopped
      * immediately.
+     *
+     * Note the message passed to this exception will be logged and therefore must be guaranteed
+     * not to contain any PII, meaning it generally can't include any information about the request
+     * URI, headers, or destination filename.
      */
     private class StopRequest extends Throwable {
         public int mFinalStatus;
 
-        public StopRequest(int finalStatus) {
+        public StopRequest(int finalStatus, String message) {
+            super(message);
             mFinalStatus = finalStatus;
         }
 
-        public StopRequest(int finalStatus, Throwable throwable) {
-            super(throwable);
+        public StopRequest(int finalStatus, String message, Throwable throwable) {
+            super(message, throwable);
             mFinalStatus = finalStatus;
         }
     }
@@ -176,15 +180,12 @@ public class DownloadThread extends Thread {
             finalStatus = Downloads.Impl.STATUS_SUCCESS;
         } catch (StopRequest error) {
             // remove the cause before printing, in case it contains PII
-            Log.w(Constants.TAG, "Aborting request for download " + mInfo.mId, removeCause(error));
+            Log.w(Constants.TAG,
+                    "Aborting request for download " + mInfo.mId + ": " + error.getMessage());
             finalStatus = error.mFinalStatus;
             // fall through to finally block
-        } catch (FileNotFoundException ex) {
-            Log.w(Constants.TAG, "FileNotFoundException for " + state.mFilename, ex);
-            finalStatus = Downloads.Impl.STATUS_FILE_ERROR;
-            // falls through to the code that reports an error
         } catch (Throwable ex) { //sometimes the socket code throws unchecked exceptions
-            Log.w(Constants.TAG, "Exception for id " + mInfo.mId, ex);
+            Log.w(Constants.TAG, "Exception for id " + mInfo.mId + ": " + ex);
             finalStatus = Downloads.Impl.STATUS_UNKNOWN_ERROR;
             // falls through to the code that reports an error
         } finally {
@@ -205,20 +206,11 @@ public class DownloadThread extends Thread {
     }
 
     /**
-     * @return an identical StopRequest but with the cause removed.
-     */
-    private StopRequest removeCause(StopRequest error) {
-        StopRequest newException = new StopRequest(error.mFinalStatus);
-        newException.setStackTrace(error.getStackTrace());
-        return newException;
-    }
-
-    /**
      * Fully execute a single download request - setup and send the request, handle the response,
      * and transfer the data to the destination file.
      */
     private void executeDownload(State state, AndroidHttpClient client, HttpGet request)
-            throws StopRequest, RetryDownload, FileNotFoundException {
+            throws StopRequest, RetryDownload {
         InnerState innerState = new InnerState();
         byte data[] = new byte[Constants.BUFFER_SIZE];
 
@@ -254,7 +246,7 @@ public class DownloadThread extends Thread {
                 status = Downloads.Impl.STATUS_QUEUED_FOR_WIFI;
                 mInfo.notifyPauseDueToSize(false);
             }
-            throw new StopRequest(status);
+            throw new StopRequest(status, mInfo.getLogMessageForNetworkError(networkUsable));
         }
     }
 
@@ -356,8 +348,8 @@ public class DownloadThread extends Thread {
         file.delete();
 
         if (item == null) {
-            Log.w(Constants.TAG, "unable to add file " + state.mFilename + " to DrmProvider");
-            throw new StopRequest(Downloads.Impl.STATUS_UNKNOWN_ERROR);
+            throw new StopRequest(Downloads.Impl.STATUS_UNKNOWN_ERROR,
+                    "unable to add file to DrmProvider");
         } else {
             state.mFilename = item.getDataString();
             state.mMimeType = item.getType();
@@ -389,17 +381,12 @@ public class DownloadThread extends Thread {
     private void checkPausedOrCanceled(State state) throws StopRequest {
         synchronized (mInfo) {
             if (mInfo.mControl == Downloads.Impl.CONTROL_PAUSED) {
-                if (Constants.LOGV) {
-                    Log.v(Constants.TAG, "paused " + mInfo.mUri);
-                }
-                throw new StopRequest(Downloads.Impl.STATUS_PAUSED_BY_APP);
+                throw new StopRequest(Downloads.Impl.STATUS_PAUSED_BY_APP,
+                        "download paused by owner");
             }
         }
         if (mInfo.mStatus == Downloads.Impl.STATUS_CANCELED) {
-            if (Constants.LOGV) {
-                Log.d(Constants.TAG, "canceled " + mInfo.mUri);
-            }
-            throw new StopRequest(Downloads.Impl.STATUS_CANCELED);
+            throw new StopRequest(Downloads.Impl.STATUS_CANCELED, "download canceled");
         }
     }
 
@@ -444,15 +431,18 @@ public class DownloadThread extends Thread {
                         continue;
                     }
                 } else if (!Helpers.isExternalMediaMounted()) {
-                    throw new StopRequest(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR);
+                    throw new StopRequest(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR,
+                            "external media not mounted while writing destination file");
                 }
 
                 long availableBytes =
                     Helpers.getAvailableBytes(Helpers.getFilesystemRoot(state.mFilename));
                 if (availableBytes < bytesRead) {
-                    throw new StopRequest(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR, ex);
+                    throw new StopRequest(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR,
+                            "insufficient space while writing destination file", ex);
                 }
-                throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR, ex);
+                throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR,
+                        "while writing destination file: " + ex.toString(), ex);
             }
         }
     }
@@ -473,16 +463,11 @@ public class DownloadThread extends Thread {
                 && (innerState.mBytesSoFar != Integer.parseInt(innerState.mHeaderContentLength));
         if (lengthMismatched) {
             if (cannotResume(innerState)) {
-                if (Constants.LOGV) {
-                    Log.d(Constants.TAG, "mismatched content length " +
-                            mInfo.mUri);
-                } else if (Config.LOGD) {
-                    Log.d(Constants.TAG, "mismatched content length for " +
-                            mInfo.mId);
-                }
-                throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME);
+                throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME,
+                        "mismatched content length");
             } else {
-                throw new StopRequest(handleHttpError(state, "closed socket"));
+                throw new StopRequest(getFinalStatusForHttpError(state),
+                        "closed socket before end of file");
             }
         }
     }
@@ -507,11 +492,13 @@ public class DownloadThread extends Thread {
             values.put(Downloads.Impl.COLUMN_CURRENT_BYTES, innerState.mBytesSoFar);
             mContext.getContentResolver().update(mInfo.getAllDownloadsUri(), values, null, null);
             if (cannotResume(innerState)) {
-                Log.d(Constants.TAG, "download IOException for download " + mInfo.mId, ex);
-                Log.d(Constants.TAG, "can't resume interrupted download with no ETag");
-                throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME, ex);
+                String message = "while reading response: " + ex.toString()
+                + ", can't resume interrupted download with no ETag";
+                throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME,
+                        message, ex);
             } else {
-                throw new StopRequest(handleHttpError(state, "download IOException"), ex);
+                throw new StopRequest(getFinalStatusForHttpError(state),
+                        "while reading response: " + ex.toString(), ex);
             }
         }
     }
@@ -526,7 +513,8 @@ public class DownloadThread extends Thread {
             return response.getEntity().getContent();
         } catch (IOException ex) {
             logNetworkState();
-            throw new StopRequest(handleHttpError(state, "IOException getting entity"), ex);
+            throw new StopRequest(getFinalStatusForHttpError(state),
+                    "while getting entity: " + ex.toString(), ex);
         }
     }
 
@@ -542,7 +530,7 @@ public class DownloadThread extends Thread {
      * file and updating the database.
      */
     private void processResponseHeaders(State state, InnerState innerState, HttpResponse response)
-            throws StopRequest, FileNotFoundException {
+            throws StopRequest {
         if (innerState.mContinuingDownload) {
             // ignore response headers on resume requests
             return;
@@ -550,22 +538,27 @@ public class DownloadThread extends Thread {
 
         readResponseHeaders(state, innerState, response);
 
-        DownloadFileInfo fileInfo = Helpers.generateSaveFile(
-                mContext,
-                mInfo.mUri,
-                mInfo.mHint,
-                innerState.mHeaderContentDisposition,
-                innerState.mHeaderContentLocation,
-                state.mMimeType,
-                mInfo.mDestination,
-                (innerState.mHeaderContentLength != null) ?
-                        Long.parseLong(innerState.mHeaderContentLength) : 0,
-                mInfo.mIsPublicApi);
-        if (fileInfo.mFileName == null) {
-            throw new StopRequest(fileInfo.mStatus);
-        }
-        state.mFilename = fileInfo.mFileName;
-        state.mStream = fileInfo.mStream;
+        try {
+            state.mFilename = Helpers.generateSaveFile(
+                    mContext,
+                    mInfo.mUri,
+                    mInfo.mHint,
+                    innerState.mHeaderContentDisposition,
+                    innerState.mHeaderContentLocation,
+                    state.mMimeType,
+                    mInfo.mDestination,
+                    (innerState.mHeaderContentLength != null) ?
+                            Long.parseLong(innerState.mHeaderContentLength) : 0,
+                    mInfo.mIsPublicApi);
+        } catch (Helpers.GenerateSaveFileError exc) {
+            throw new StopRequest(exc.mStatus, exc.mMessage);
+        }
+        try {
+            state.mStream = new FileOutputStream(state.mFilename);
+        } catch (FileNotFoundException exc) {
+            throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR,
+                    "while opening destination file: " + exc.toString(), exc);
+        }
         if (Constants.LOGV) {
             Log.v(Constants.TAG, "writing " + mInfo.mUri + " to " + state.mFilename);
         }
@@ -647,8 +640,8 @@ public class DownloadThread extends Thread {
                 && (headerTransferEncoding == null
                     || !headerTransferEncoding.equalsIgnoreCase("chunked"));
         if (!mInfo.mNoIntegrity && noSizeInfo) {
-            Log.d(Constants.TAG, "can't know size of download, giving up");
-            throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR);
+            throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
+                    "can't know size of download, giving up");
         }
     }
 
@@ -676,12 +669,6 @@ public class DownloadThread extends Thread {
      */
     private void handleOtherStatus(State state, InnerState innerState, int statusCode)
             throws StopRequest {
-        if (Constants.LOGV) {
-            Log.d(Constants.TAG, "http error " + statusCode + " for " + mInfo.mUri);
-        } else if (Config.LOGD) {
-            Log.d(Constants.TAG, "http error " + statusCode + " for download " +
-                    mInfo.mId);
-        }
         int finalStatus;
         if (Downloads.Impl.isStatusError(statusCode)) {
             finalStatus = statusCode;
@@ -692,7 +679,7 @@ public class DownloadThread extends Thread {
         } else {
             finalStatus = Downloads.Impl.STATUS_UNHANDLED_HTTP_CODE;
         }
-        throw new StopRequest(finalStatus);
+        throw new StopRequest(finalStatus, "http error " + statusCode);
     }
 
     /**
@@ -704,13 +691,7 @@ public class DownloadThread extends Thread {
             Log.v(Constants.TAG, "got HTTP redirect " + statusCode);
         }
         if (state.mRedirectCount >= Constants.MAX_REDIRECTS) {
-            if (Constants.LOGV) {
-                Log.d(Constants.TAG, "too many redirects for download " + mInfo.mId +
-                        " at " + mInfo.mUri);
-            } else if (Config.LOGD) {
-                Log.d(Constants.TAG, "too many redirects for download " + mInfo.mId);
-            }
-            throw new StopRequest(Downloads.Impl.STATUS_TOO_MANY_REDIRECTS);
+            throw new StopRequest(Downloads.Impl.STATUS_TOO_MANY_REDIRECTS, "too many redirects");
         }
         Header header = response.getFirstHeader("Location");
         if (header == null) {
@@ -727,12 +708,9 @@ public class DownloadThread extends Thread {
             if (Constants.LOGV) {
                 Log.d(Constants.TAG, "Couldn't resolve redirect URI " + header.getValue()
                         + " for " + mInfo.mUri);
-            } else if (Config.LOGD) {
-                Log.d(Constants.TAG,
-                        "Couldn't resolve redirect URI for download " +
-                        mInfo.mId);
             }
-            throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR);
+            throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
+                    "Couldn't resolve redirect URI");
         }
         ++state.mRedirectCount;
         state.mRequestUri = newUri;
@@ -773,7 +751,8 @@ public class DownloadThread extends Thread {
                // ignored - retryAfter stays 0 in this case.
            }
         }
-        throw new StopRequest(Downloads.Impl.STATUS_WAITING_TO_RETRY);
+        throw new StopRequest(Downloads.Impl.STATUS_WAITING_TO_RETRY,
+                "got 503 Service Unavailable, will retry later");
     }
 
     /**
@@ -784,36 +763,23 @@ public class DownloadThread extends Thread {
         try {
             return client.execute(request);
         } catch (IllegalArgumentException ex) {
-            if (Constants.LOGV) {
-                Log.d(Constants.TAG, "Arg exception trying to execute request for " +
-                        mInfo.mUri + " : " + ex);
-            } else if (Config.LOGD) {
-                Log.d(Constants.TAG, "Arg exception trying to execute request for " +
-                        mInfo.mId + " : " +  ex);
-            }
-            throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR, ex);
+            throw new StopRequest(Downloads.Impl.STATUS_HTTP_DATA_ERROR,
+                    "while trying to execute request: " + ex.toString(), ex);
         } catch (IOException ex) {
             logNetworkState();
-            throw new StopRequest(handleHttpError(state, "IOException trying to execute request"),
-                    ex);
+            throw new StopRequest(getFinalStatusForHttpError(state),
+                    "while trying to execute request: " + ex.toString(), ex);
         }
     }
 
-    /**
-     * @return the final status for this attempt
-     */
-    private int handleHttpError(State state, String message) {
-        if (Constants.LOGV) {
-            Log.d(Constants.TAG, message + " for " + mInfo.mUri);
-        }
-
+    private int getFinalStatusForHttpError(State state) {
         if (!Helpers.isNetworkAvailable(mSystemFacade)) {
             return Downloads.Impl.STATUS_WAITING_FOR_NETWORK;
         } else if (mInfo.mNumFailed < Constants.MAX_RETRIES) {
             state.mCountRetry = true;
             return Downloads.Impl.STATUS_WAITING_TO_RETRY;
         } else {
-            Log.d(Constants.TAG, "reached max retries: " + message + " for " + mInfo.mId);
+            Log.w(Constants.TAG, "reached max retries for " + mInfo.mId);
             return Downloads.Impl.STATUS_HTTP_DATA_ERROR;
         }
     }
@@ -823,10 +789,12 @@ public class DownloadThread extends Thread {
      * appropriately for resumption.
      */
     private void setupDestinationFile(State state, InnerState innerState)
-            throws StopRequest, FileNotFoundException {
+            throws StopRequest {
         if (state.mFilename != null) { // only true if we've already run a thread for this download
             if (!Helpers.isFilenameValid(state.mFilename)) {
-                throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR);
+                // this should never happen
+                throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR,
+                        "found invalid internal destination filename");
             }
             // We're resuming a download that got interrupted
             File f = new File(state.mFilename);
@@ -838,12 +806,17 @@ public class DownloadThread extends Thread {
                     state.mFilename = null;
                 } else if (mInfo.mETag == null && !mInfo.mNoIntegrity) {
                     // This should've been caught upon failure
-                    Log.wtf(Constants.TAG, "Trying to resume a download that can't be resumed");
                     f.delete();
-                    throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME);
+                    throw new StopRequest(Downloads.Impl.STATUS_CANNOT_RESUME,
+                            "Trying to resume a download that can't be resumed");
                 } else {
                     // All right, we'll be able to resume this download
-                    state.mStream = new FileOutputStream(state.mFilename, true);
+                    try {
+                        state.mStream = new FileOutputStream(state.mFilename, true);
+                    } catch (FileNotFoundException exc) {
+                        throw new StopRequest(Downloads.Impl.STATUS_FILE_ERROR,
+                                "while opening destination for resuming: " + exc.toString(), exc);
+                    }
                     innerState.mBytesSoFar = (int) fileLength;
                     if (mInfo.mTotalBytes != -1) {
                         innerState.mHeaderContentLength = Long.toString(mInfo.mTotalBytes);
diff --git a/src/com/android/providers/downloads/Helpers.java b/src/com/android/providers/downloads/Helpers.java
index f1f23b17..cbcae5f4 100644
--- a/src/com/android/providers/downloads/Helpers.java
+++ b/src/com/android/providers/downloads/Helpers.java
@@ -33,8 +33,6 @@ import android.util.Log;
 import android.webkit.MimeTypeMap;
 
 import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
 import java.util.Random;
 import java.util.Set;
 import java.util.regex.Matcher;
@@ -75,18 +73,20 @@ public class Helpers {
     /**
      * Exception thrown from methods called by generateSaveFile() for any fatal error.
      */
-    private static class GenerateSaveFileError extends Exception {
+    public static class GenerateSaveFileError extends Exception {
         int mStatus;
+        String mMessage;
 
-        public GenerateSaveFileError(int status) {
+        public GenerateSaveFileError(int status, String message) {
             mStatus = status;
+            mMessage = message;
         }
     }
 
     /**
-     * Creates a filename (where the file should be saved) from a uri.
+     * Creates a filename (where the file should be saved) from info about a download.
      */
-    public static DownloadFileInfo generateSaveFile(
+    public static String generateSaveFile(
             Context context,
             String url,
             String hint,
@@ -95,40 +95,31 @@ public class Helpers {
             String mimeType,
             int destination,
             long contentLength,
-            boolean isPublicApi) throws FileNotFoundException {
-
-        if (!canHandleDownload(context, mimeType, destination, isPublicApi)) {
-            return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE);
-        }
-
-        String fullFilename;
-        try {
-            if (destination == Downloads.Impl.DESTINATION_FILE_URI) {
-                fullFilename = getPathForFileUri(hint, contentLength);
-            } else {
-                fullFilename = chooseFullPath(context, url, hint, contentDisposition,
-                                              contentLocation, mimeType, destination,
-                                              contentLength);
-            }
-        } catch (GenerateSaveFileError exc) {
-            return new DownloadFileInfo(null, null, exc.mStatus);
+            boolean isPublicApi) throws GenerateSaveFileError {
+        checkCanHandleDownload(context, mimeType, destination, isPublicApi);
+        if (destination == Downloads.Impl.DESTINATION_FILE_URI) {
+            return getPathForFileUri(hint, contentLength);
+        } else {
+            return chooseFullPath(context, url, hint, contentDisposition, contentLocation, mimeType,
+                    destination, contentLength);
         }
-
-        return new DownloadFileInfo(fullFilename, new FileOutputStream(fullFilename), 0);
     }
 
     private static String getPathForFileUri(String hint, long contentLength)
             throws GenerateSaveFileError {
         if (!isExternalMediaMounted()) {
-            throw new GenerateSaveFileError(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR,
+                    "external media not mounted");
         }
         String path = Uri.parse(hint).getPath();
         if (new File(path).exists()) {
             Log.d(Constants.TAG, "File already exists: " + path);
-            throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ALREADY_EXISTS_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ALREADY_EXISTS_ERROR,
+                    "requested destination file already exists");
         }
         if (getAvailableBytes(getFilesystemRoot(path)) < contentLength) {
-            throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR,
+                    "insufficient space on external storage");
         }
 
         return path;
@@ -179,19 +170,17 @@ public class Helpers {
         return chooseUniqueFilename(destination, filename, extension, recoveryDir);
     }
 
-    private static boolean canHandleDownload(Context context, String mimeType, int destination,
-            boolean isPublicApi) {
+    private static void checkCanHandleDownload(Context context, String mimeType, int destination,
+            boolean isPublicApi) throws GenerateSaveFileError {
         if (isPublicApi) {
-            return true;
+            return;
         }
 
         if (destination == Downloads.Impl.DESTINATION_EXTERNAL
                 || destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) {
             if (mimeType == null) {
-                if (Config.LOGD) {
-                    Log.d(Constants.TAG, "external download with no mime type not allowed");
-                }
-                return false;
+                throw new GenerateSaveFileError(Downloads.Impl.STATUS_NOT_ACCEPTABLE,
+                        "external download with no mime type not allowed");
             }
             if (!DrmRawContent.DRM_MIMETYPE_MESSAGE_STRING.equalsIgnoreCase(mimeType)) {
                 // Check to see if we are allowed to download this file. Only files
@@ -212,14 +201,14 @@ public class Helpers {
                 //Log.i(Constants.TAG, "*** FILENAME QUERY " + intent + ": " + list);
 
                 if (ri == null) {
-                    if (Config.LOGD) {
-                        Log.d(Constants.TAG, "no handler found for type " + mimeType);
+                    if (Constants.LOGV) {
+                        Log.v(Constants.TAG, "no handler found for type " + mimeType);
                     }
-                    return false;
+                    throw new GenerateSaveFileError(Downloads.Impl.STATUS_NOT_ACCEPTABLE,
+                            "no handler found for this download type");
                 }
             }
         }
-        return true;
     }
 
     private static File locateDestinationDirectory(Context context, String mimeType,
@@ -239,23 +228,24 @@ public class Helpers {
 
     private static File getExternalDestination(long contentLength) throws GenerateSaveFileError {
         if (!isExternalMediaMounted()) {
-            throw new GenerateSaveFileError(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR,
+                    "external media not mounted");
         }
 
         File root = Environment.getExternalStorageDirectory();
         if (getAvailableBytes(root) < contentLength) {
             // Insufficient space.
             Log.d(Constants.TAG, "download aborted - not enough free space");
-            throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR,
+                    "insufficient space on external media");
         }
 
         File base = new File(root.getPath() + Constants.DEFAULT_DL_SUBDIR);
         if (!base.isDirectory() && !base.mkdir()) {
             // Can't create download directory, e.g. because a file called "download"
             // already exists at the root level, or the SD card filesystem is read-only.
-            Log.d(Constants.TAG, "download aborted - can't create base directory "
-                    + base.getPath());
-            throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR,
+                    "unable to create external downloads directory " + base.getPath());
         }
         return base;
     }
@@ -278,9 +268,9 @@ public class Helpers {
             // Insufficient space; try discarding purgeable files.
             if (!discardPurgeableFiles(context, contentLength - bytesAvailable)) {
                 // No files to purge, give up.
-                Log.d(Constants.TAG,
-                        "download aborted - not enough free space in internal storage");
-                throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
+                throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR,
+                        "not enough free space in internal download storage, unable to free any "
+                        + "more");
             }
             bytesAvailable = getAvailableBytes(base);
         }
@@ -482,7 +472,8 @@ public class Helpers {
                 sequence += sRandom.nextInt(magnitude) + 1;
             }
         }
-        throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR);
+        throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR,
+                "failed to generate an unused filename on internal download storage");
     }
 
     /**