Closed Bug 1141757 Opened 8 years ago Closed 8 years ago

Non-intermittent "Full message: TypeError: doc.documentElement is undefined" errors causing Talos bustage to be misstarred

Categories

(Firefox Graveyard :: Reading List, defect)

Product:
Firefox Graveyard
Component:
Reading List
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox38 fixed, firefox39 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 39
Tracking Flags:
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: RyanVM, Assigned: Margaret)

References

Details

Attachments

(1 file)

Catch exceptions thrown in ReaderWorker
1.32 KB, patch
Yoric
: review+
Details | Diff | Splinter Review 
12:46:24 INFO - ReaderWorker: Received message [object Object]
12:46:24 INFO - ReaderWorker: Calling method parseDocument
12:46:24 ERROR - JSDOMParser error: expected '</lineargradient>'JSDOMParser error: expected '</defs>'JSDOMParser error: expected '</svg>'ReaderWorker: Error while calling agent method parseDocument TypeError: doc.documentElement is undefined Readability.prototype._grabArticle@resource://gre/modules/reader/Readability.js:418:5
12:46:24 INFO - Readability.prototype.parse@resource://gre/modules/reader/Readability.js:1507:26
12:46:24 INFO - Agent.parseDocument@resource://gre/modules/reader/ReaderWorker.js:44:12
12:46:24 INFO - worker.dispatch@resource://gre/modules/reader/ReaderWorker.js:19:24
12:46:24 INFO - anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16
12:46:24 INFO - @resource://gre/modules/reader/ReaderWorker.js:31:41
12:46:24 INFO - ReaderWorker: Sending back exception TypeError id is 1
12:46:24 INFO - *************************
12:46:24 INFO - A coding exception was thrown and uncaught in a Task.
12:46:24 ERROR - Full message: TypeError: doc.documentElement is undefined
12:46:24 INFO - Full stack: Readability.prototype._grabArticle@resource://gre/modules/reader/Readability.js:418:5
12:46:24 INFO - Readability.prototype.parse@resource://gre/modules/reader/Readability.js:1507:26
12:46:24 INFO - Agent.parseDocument@resource://gre/modules/reader/ReaderWorker.js:44:12
12:46:24 INFO - worker.dispatch@resource://gre/modules/reader/ReaderWorker.js:19:24
12:46:24 INFO - anonymous/AbstractWorker.prototype.handleMessage@resource://gre/modules/workers/PromiseWorker.js:122:16
12:46:24 INFO - @resource://gre/modules/reader/ReaderWorker.js:31:41
12:46:24 INFO - *************************
12:46:24 INFO - *************************
12:46:24 INFO - A coding exception was thrown and uncaught in a Task.
The first one was a hang while posting to graphserver, 3600 seconds without output. Comment 4 was an as-yet-unfiled xperf mainthreadio failure. The rest have been bug 1134790 on 10.10.

This isn't a failure, it's present in every green run of affected suites, it just causes us to fail to see (and tbplbot to fail to fit into comments) the actual error in failing runs.
Summary: Intermittent "Full message: TypeError: doc.documentElement is undefined" errors causing Talos bustage → Non-intermittent "Full message: TypeError: doc.documentElement is undefined" errors causing Talos bustage to be misstarred
Caused by https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=c186a66948fb which is rather inconvenient since that just says that it's caused by having Reader enabled.
Blocks: 1140045
I'll take a look. Hopefully I can track this down by just reasoning about the code.
Assignee: nobody → margaret.leibovic
So, there are 2 issues here:

1) There is obviously some error in Readability.js. We should fix that.
2) An error in Readbaility.js isn't being properly handled. I don't think we can depend on Readability.js never having problems, so we need to handle this.

This sounds like something new from my switch to use PromiseWorkers (bug 1140172). I'll write a patch here to handle catching the error.

        Attachment #8576050 -
        Flags: review?(dteller)

    
    

    
  
Comment on attachment 8576050 [details] [diff] [review]
Catch exceptions thrown in ReaderWorker

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

Is this error designed for developers of power users who by chance read the browser console?
If the former, we need a stack trace.

        Attachment #8576050 -
        Flags: review?(dteller) → review+

    
    

    
  
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #14)
> Comment on attachment 8576050 [details] [diff] [review]
> Catch exceptions thrown in ReaderWorker
> 
> Review of attachment 8576050 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is this error designed for developers of power users who by chance read the
> browser console?
> If the former, we need a stack trace.

I would say both, as we want the power users to report bugs. I can add a stack trace. Do you think it's okay to include a stack in Cu.reportError?
Flags: needinfo?(dteller)

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
I may be getting rid of this worker to address some memory problems, and as it is, we don't include stacks in errors we report in the reader code, so I just decided to land this as it is:

https://hg.mozilla.org/integration/fx-team/rev/1b2838891609
Flags: needinfo?(dteller)

    
  
Blocks: 1140172

    
    

    
      
  

    
    

    
      
  

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/1b2838891609
Status: NEW → RESOLVED
Closed: 8 years ago

          status-firefox39:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39

    
    

    
  
Setting qe-verify- for this fix. Margaret, if you think there something manual QA should look at here, please flip that flag.
Flags: qe-verify-

    
  
Keywords: intermittent-failure
Product: Firefox → Firefox Graveyard

          You need to log in
          before you can comment on or make changes to this bug.