Closed Bug 790496 Opened 7 years ago Closed 7 years ago

Make maintenance service errors stand out more

Categories

(Toolkit :: Application Update, defect)

18 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: bbondy, Assigned: bbondy)

References

Details

Attachments

(1 file, 3 obsolete files)

We should make errors in the maintenance service logs look more scary by prepending and appending some *** to the lines that are errors.  A small change like this probably would have allowed us to find bug 789743 before it reached the release channel.
Attached patch Patch v1. (obsolete) — Splinter Review
Attachment #660306 - Flags: review?(ehsan)
Comment on attachment 660306 [details] [diff] [review]
Patch v1.

Review of attachment 660306 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/update/common/updatelogging.h
@@ +34,5 @@
>    FILE *logFP;
>    NS_tchar* sourcePath;
>  };
>  
> +#define LOG_WARN_LN(args) UpdateLog::GetPrimaryLog().WarnPrintfLn args

I'm not a huge fan of the _LN postfix, so please drop it if you agree.
Attachment #660306 - Flags: review?(ehsan) → review+
ya that's fine, I just put it to differentiate from LOG(( which actually requires a manually added \n
(In reply to comment #3)
> ya that's fine, I just put it to differentiate from LOG(( which actually
> requires a manually added \n

Then looks like we should fix LOG.  ;-)
I'm not sure if my fingers can fix anymore LOG( calls to remove \n :)
Attached patch Patch v2. (obsolete) — Splinter Review
Changed a couple hundred lines so thought I should re-ask for review.
- Added some GetLastError() where they were missing from maintenanceservice stuff
- Made formatting of last error reporting more consistent
- Removed some trailing spaces
- Removed \n from all the LOG(( lines via global search/replace
- added a \n output to the function that is called in the LOG macro
Attachment #660306 - Attachment is obsolete: true
Attachment #660630 - Flags: review?(ehsan)
Attachment #660630 - Flags: review?(ehsan) → review+
Attached patch Patch v3. (obsolete) — Splinter Review
Rebased. Carrying forward r+.
Attachment #660630 - Attachment is obsolete: true
Attachment #674742 - Flags: review+
Attached patch Patch v4.Splinter Review
%s/WarnPrintfLn/WarnPrintf in the .cpp
Attachment #674742 - Attachment is obsolete: true
Attachment #674786 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/612a40a222d7
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.