While trying to fix "Bug 930397 - Failure to handle error during writing to an almost full file system (TB saving an attachment/FF downloading)", I noticed that the error of failing PR_Write() is not propagated properly to callers in nsLocalFile::CopyToNative (File: mozilla/xpcom/io/nsLocalFileCommon.cpp ) This makes it impossible for the upper layer caller to display meaningful error message to the user. Currently, a bland catch-all message ("The download cannot be saved because an unknown error occurred.") is shown and users are left with no clue to the cause of the error. But then, we need a mapping of POSIX errno to NS_ERROR_* type macros to pass proper error information to the callers. In mozilla/xpcom/io/nsLocalFileCommon.cpp, I noticed that nsDownload::CopyMoveFailed() used a macro #define NSRESULT_FOR_ERRNO() nsresultForErrno(errno) defined in FILE: mozilla/xpcom/io/nsLocalFile.h to map the value of errno to NS_ERROR_* macros. So I thought I could use this macro when write failed to write expected number of octets to a local file (is the typical symptom of failure to the filled up file system during writing.) To my dismay, I found out that the macro, and nsresultForErrno() function in turn, does not implement all the POSIX error values :-( I do NOT NEED ALL the POSIX error macros, I only need macros for often seen errors to be handled by nsresultForErrno(errno) for now. Specifically, for an error scenario, in Bug 930397, I definitely need the mapping of ENOSPC (e.g., defined under linux as follows.) #define ENOSPC 28 /* No space left on device */ So I decided to add handling of some additional POSIX error macro values to nsresultForErrno(). In adding ENOSPC to the values handled nsresultForErrno(), A few more POSIX error macros that may be relevant to FILE I?O error handling are added also. A patch to the function follows. TIA PS: In doing the above, I noticed a rather unfortunate misguided usage of NS_ERROR_FILE_TOO_BIG by createUnique() function: I reported the bug and proposed a patch so that NS_ERROR_FILE_ALREADY_EXISTS is used instead : Bug 931689 - Strange use of NS_ERROR_FILE_TOO_BIG by nsLocalFile::CreateUnique: NS_FILE_ALREADY_EXISTS should be used instead. With the change in Bug 931689, I can universally convert POSIX EFBIG to NS_ERROR_FILE_TOO_BIG for the whole the comm-central tree (and thus the same in the mozilla source tree.) PPS: this is valid for all POSIX-compliant OSs including MacOSX, I think. (Or solaris and FreeBSD for that matter.)
Created attachment 823226 [details] [diff] [review] PATCH: Adding the processing of a few POSIX errors macros. This is the patch. The lack of handling of these important additional POSIX error macros may explain the lack of user-friendly detailed error handling in TB (and FF maybe, too.) I hope this gets in soon. But I am not sure who the reviewer should be. TIA
Comment on attachment 823226 [details] [diff] [review] PATCH: Adding the processing of a few POSIX errors macros. r=me except for the comment about CreateUnique, which needs to continue returning NS_ERROR_FILE_TOO_BIG.
Attachment #823226 - Flags: review?(benjamin) → review+
Created attachment 825105 [details] [diff] [review] FINAL PATCH: Adding the processing of a few POSIX errors macros. (carrying over r= bsmedberg) Thank you for the review. I have changed the description to be a note for the newcomer. (I think a comment is necessary here to avoid future misunderstanding.) If this is OK, then I will set checkin-needed keyword. TIA
Thank you for the review. I will put in the checkin-needed keyword.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.