Closed Bug 629664 Opened 13 years ago Closed 13 years ago

Windows opt. TEST-UNEXPECTED-FAIL | test_service_sync_checkServerError.js | test failed (with xpcshell return code: 0), see following log, followed by, TEST-UNEXPECTED-FAIL | (xpcshell/he‌ad.js) | TypeError: engine is undefined - See following stack:

Categories

(Firefox :: Sync, defect)

Product:
Firefox
Component:
Sync
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jwatt, Assigned: philikon)

References

Details

(Keywords: intermittent-failure)

Attachments

(4 files)

Make test execution async
5.89 KB, patch
rnewman
: review+
Details | Diff | Splinter Review
Proposed patch, part 1. v1
5.94 KB, patch
philikon
: review+
Details | Diff | Splinter Review
Proposed patch, part 2. v1
5.29 KB, patch
philikon
: review+
Details | Diff | Splinter Review
re-fix
3.47 KB, patch
rnewman
: review+
Details | Diff | Splinter Review 
This has happened on WinXP opt three times so far.

This first time it occurred on Fri Jan 28 21:21:10 2011:
http://tbpl.mozilla.org/?rev=7eac2da4ca5e

Four pushes later it occurred again on Fri Jan 28 22:25:18 2011
http://tbpl.mozilla.org/?rev=3cbf026c2ce2

Three pushes later it occurred on Sat Jan 29 00:27:29 2011
http://tbpl.mozilla.org/?rev=a84260b56b08

Details for the first occurrence:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1296215964.1296217593.3721.gz&fulltext=1
Rev3 WINNT 5.1 mozilla-central opt test xpcshell on 2011/01/28 03:59:24

s: talos-r3-xp-014
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\xpcshell\tests\services\sync\tests\unit\test_service_sync_checkServerError.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | TypeError: engine is undefined - See following stack:
Blocks: 438871
Component: General → Firefox Sync: Backend
Product: Core → Mozilla Services
QA Contact: general → sync-backend
Thanks, Jonathan. I'll see what's up.
Assignee: nobody → philipp
Cool, thanks.

Another thing. There were four builds with this "TypeError: engine is undefined" thing, but tbpl only put this bug in the suggestion list for one of them. The three that it didn't were already starred (I think), but I'm still wondering why it wouldn't suggest this bug when it was still suggesting other bugs.

Oh, and when I starred those four build with this bug's bug number, TinderboxPushlog Robot only reported one of those here. Again than seems like there's a problem, and I'm curious what it might be if anyone knows.
Let's see if that fixes it.
Attachment #507979 - Flags: review?(rnewman)
Comment on attachment 507979 [details] [diff] [review]
Make test execution async

Looks good, tests pass locally. Fingers crossed!
Attachment #507979 - Flags: review?(rnewman) → review+
Landed on fx-sync: https://hg.mozilla.org/services/fx-sync/rev/09faa22076d3

Will resolve once it's merged to m-c.
Blocks: 629603
Blast, that patch didn't do it. Will have to look closer whether we're racing something else here, or just missing something shamefully obvious.
Summary: WinXP opt. TEST-UNEXPECTED-FAIL | test_service_sync_checkServerError.js | test failed (with xpcshell return code: 0), see following log, followed by, TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | TypeError: engine is undefined - See following stack: → Windows opt. TEST-UNEXPECTED-FAIL | test_service_sync_checkServerError.js | test failed (with xpcshell return code: 0), see following log, followed by, TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | TypeError: engine is undefined - See following stack:
Here's some cleanup of the tests...
Attachment #511259 - Flags: review?(philipp)
... and only register the test engine once.
Attachment #511260 - Flags: review?(philipp)
Kicked off a try build with these changes:

http://tbpl.mozilla.org/?tree=MozillaTry&rev=9700ce5c9a45
Comment on attachment 511259 [details] [diff] [review]
Proposed patch, part 1. v1

>+  // Track these using the collections helper, which keeps modified times
>+  // up-to-date.
>+  let keysWBO     = new ServerWBO("keys");
>+  let clientsColl = new ServerCollection({}, true);
>+  let globalWBO   = new ServerWBO("global", {storageVersion: STORAGE_VERSION,
>+                                             syncID: "AAAAAAAA"});

You could use Utils.makeGUID() here since we don't seem to depend on this magic value "AAAAAAAA". More to the point, however, don't you need the 'engines' object in there as well? E.g.:

  engines: {catapult: {version: engine.version, syncID: engine.syncID}}

  (This requires that the engine is already registered at this point and engine = Engines.get("catapult").)

Otherwise I'd assume the service disables your engine during sync. Maybe I'm wrong, but it seems suspicious to me. Please double check.

Looks good otherwise.
Attachment #511259 - Flags: review?(philipp) → review+
Comment on attachment 511260 [details] [diff] [review]
Proposed patch, part 2. v1

>+                  function cleanup(next) {
>+                    Engines.unregister("catapult");
>+                    next();
>+                  },
>                   do_test_finished)();
> }

No need to clean up here. Process ends as soon as do_test_finished returns.
Attachment #511260 - Flags: review?(philipp) → review+
(In reply to comment #46)
> Comment on attachment 511259 [details] [diff] [review]
> Proposed patch, part 1. v1
> 
> >+  // Track these using the collections helper, which keeps modified times
> >+  // up-to-date.
> >+  let keysWBO     = new ServerWBO("keys");
> >+  let clientsColl = new ServerCollection({}, true);
> >+  let globalWBO   = new ServerWBO("global", {storageVersion: STORAGE_VERSION,
> >+                                             syncID: "AAAAAAAA"});
> 
> You could use Utils.makeGUID() here...

Done.

> More to the point, however, don't you need the 'engines'
> object in there as well? E.g.:

Actually, no, but only by accident! See:

Service.Main	INFO	One client and no enabled engines: not touching local engine status.

Fixed.

 
> Looks good otherwise.

Great, thanks. Try build completed without error, so looks positive...

Review comment amendments made, so pushing.
Pushed:

http://hg.mozilla.org/services/fx-sync/rev/aee94a4244ad
http://hg.mozilla.org/services/fx-sync/rev/db531f6e2935

Going to close this; we'll reopen if it reoccurs. Again.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Actually, reopening this until it's landed. See, this is what happens when I work after beer time.

Philipp, OK for me to land this on Places, or should I leave that to the Merge Viking?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [orange] → [orange][fixed in fx-sync]
Whiteboard: [orange][fixed in fx-sync] → [orange][fixed in fx-sync][fixed in places]
Landed part 1 and part 2 on m-c:

http://hg.mozilla.org/mozilla-central/rev/fd42b5a411aa
http://hg.mozilla.org/mozilla-central/rev/6c2446bf14ee

The orangefactor has been hovering around 10% [1] so let's leave this open for now gather some statistics. If we get no more failures after a day or two, we can close it.

[1] http://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=629664
(In reply to comment #56)
> The orangefactor has been hovering around 10% [1] so let's leave this open for
> now gather some statistics. If we get no more failures after a day or two, we
> can close it.

No more occurrences. Closing.
Status: NEW → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
I'm an idiot. Marina and I unfixed this when we wrote the test for bug 624436. Will re-fix it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [orange][fixed in fx-sync][fixed in places] → [orange]
Attached patch re-fix — — Splinter Review 

        Attachment #534022 -
        Flags: review?(rnewman)

        Attachment #534022 -
        Flags: review?(rnewman) → review+

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Merged re-fix: http://hg.mozilla.org/mozilla-central/rev/78299a6f7e1d
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED

    
  
Summary: Windows opt. TEST-UNEXPECTED-FAIL | test_service_sync_checkServerError.js | test failed (with xpcshell return code: 0), see following log, followed by, TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | TypeError: engine is undefined - See following stack: Windo… → Windows opt. TEST-UNEXPECTED-FAIL | test_service_sync_checkServerError.js | test failed (with xpcshell return code: 0), see following log, followed by, TEST-UNEXPECTED-FAIL | (xpcshell/he‌ad.js) | TypeError: engine is undefined - See following stack Win…
Whiteboard: [orange]
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: