Add proper console logs when app cache manifest load failes

RESOLVED FIXED in mozilla19

Status

()

Core
Networking: Cache
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mayhemer, Assigned: mayhemer)

Tracking

Trunk
mozilla19
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(b2g18 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
I on and on receive bug mails where people are asking why the hell their offline app cache is not offline cached.  To figure out I usually have to process logs.

Hence, I will add proper logs when an error occurs during manifest load or processing as warnings or infos in the error console for:

- manifest has not been changed, not updating cache [info]
- manifest removed from the server, cache cleared [info]
- manifest request failed with error X, cache load failed [warning]
- manifest failed to parse on item N, cache load failed [warning]
- manifest item N failed to load with error X, cache load failed [warning]
- manifest item N is a redirect, cache load failed [warning]
- manifest N updated successfully [info]
(In reply to Honza Bambas (:mayhemer) from comment #0)
>  [...] in the error console [...]:

AFAICT, such errors should be put into the web console, not the error console. One reason is private browsing. The other reason is the long-term goal is for the error console to go away (the menu item for it has already been removed, recently). The most compelling reason, though, is that developers expect networking error messages for the current tab to be in the web console.
(Assignee)

Comment 2

5 years ago
(In reply to Brian Smith (:bsmith) from comment #1)
> (In reply to Honza Bambas (:mayhemer) from comment #0)
> >  [...] in the error console [...]:
> 
> AFAICT, such errors should be put into the web console, not the error
> console. One reason is private browsing. The other reason is the long-term
> goal is for the error console to go away (the menu item for it has already
> been removed, recently). The most compelling reason, though, is that
> developers expect networking error messages for the current tab to be in the
> web console.

I was expecting nsIConsoleService logged to web console, but it's not.

Better would be to just add offline cache update requests to the load group and that would automatically log them all to the web console what may give enough clue.  

I just work on that, since logging is not what I actually want finally. (I may close this as WONTFIX eventually).
(Assignee)

Comment 3

5 years ago
Adding all requests to the document's load group is not the way here.  When user cancels the load or just changes the URL (we also do this in offline tests) then offline cache loads are canceled and the update process fails.  That is against the spec.  Offline cache loads also must not influence the page load indicator and e.g. mixed content or security UI.

Brian, do you know how to log to the Web Console?  I cannot find out how its API works.  I have a patch to log to Error Console, I think we can change it simply to log to Web Console later.

I'll submit the patch and make it to land to have at least something quickly now.
(Assignee)

Comment 4

5 years ago
Created attachment 677445 [details] [diff] [review]
v1
Attachment #677445 - Flags: review?(jduell.mcbugs)

Comment 5

5 years ago
Web consoles look for nsIScriptError objects that have window IDs that are non-zero.

Comment 6

5 years ago
In other words, use ReportScriptError instead of LogStringMessage and pass a relevant window ID to the script error Init method.
(In reply to Josh Matthews [:jdm] from comment #6)
> In other words, use ReportScriptError instead of LogStringMessage and pass a
> relevant window ID to the script error Init method.

Necko channels shouldn't know anything about windows.

I suggest that we add a logging interface that the notification callbacks can implement. Then, have nsDocShell implement that logging interface by getting the window ID and then ReportScriptError with it. AFAICT, this will allow any/all developer tools and addons to hook into the logging to report it where necessary.
(Assignee)

Comment 8

5 years ago
(In reply to Brian Smith (:bsmith) from comment #7)
> (In reply to Josh Matthews [:jdm] from comment #6)
> > In other words, use ReportScriptError instead of LogStringMessage and pass a
> > relevant window ID to the script error Init method.
> 
> Necko channels shouldn't know anything about windows.
> 

Brian, I want to add the logs to the offline cache update code that knows (has to) windows.

> I suggest that we add a logging interface that the notification callbacks
> can implement. Then, have nsDocShell implement that logging interface by
> getting the window ID and then ReportScriptError with it. AFAICT, this will
> allow any/all developer tools and addons to hook into the logging to report
> it where necessary.

I don't agree with anything complicated.  I want right now a simple way to see what happens.  Rather then wait until somebody (because I don't have time) finish the machine you propose.  Report a new bug for that please.
(Assignee)

Comment 9

5 years ago
(In reply to Josh Matthews [:jdm] from comment #6)
> In other words, use ReportScriptError instead of LogStringMessage and pass a
> relevant window ID to the script error Init method.

What API do you have in mind?
I wasn't thinking; I forgot that nsIScriptError inherits from nsIConsoleMessage. The nsIConsoleService stuff works fine.
Comment on attachment 677445 [details] [diff] [review]
v1

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

::: uriloader/prefetch/nsOfflineCacheUpdate.cpp
@@ +81,5 @@
>      uint32_t mCount;
>      char **mValues;
>  };
>  
> +namespace { // anon

I really dislike anonymous namespaces.  See my rant on dev.platform.  I'd guess the result will be that you should feel free to use them :)
Attachment #677445 - Flags: review?(jduell.mcbugs) → review+
(Assignee)

Comment 12

5 years ago
(In reply to Jason Duell (:jduell) from comment #11)
> I really dislike anonymous namespaces.  

I don't like so many things in our styling guild...

RE namespaces: use visual studio ;)
(Assignee)

Comment 13

5 years ago
Comment on attachment 677445 [details] [diff] [review]
v1

https://hg.mozilla.org/integration/mozilla-inbound/rev/b97258fb92ba

I left the namespace.  If something changes in this manner, then it will be a big replace anyway.
Attachment #677445 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/b97258fb92ba
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment on attachment 677445 [details] [diff] [review]
v1

Just adds some debugging console infrastructure, which is used by b2g blocker bug 761040
Attachment #677445 - Flags: approval-mozilla-b2g18?
Attachment #677445 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
(Assignee)

Comment 17

5 years ago
(In reply to Jason Duell (:jduell) from comment #16)
> https://hg.mozilla.org/releases/mozilla-b2g18/rev/b63d9e25cb6f

Thanks!  I didn't realize it didn't ever land on b2g18.
(Assignee)

Updated

5 years ago
Duplicate of this bug: 635973
You need to log in before you can comment on or make changes to this bug.