Closed Bug 1189144 Opened 9 years ago Closed 9 years ago

Intermittent fxa_launch_test.js | Firefox Accounts Launch Tests Should launch FxA flow from FxA-consuming apps: Settings/UITest/test-fxa-client app

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: RyanVM, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Jared, any idea what might have happened here?
Flags: needinfo?(6a68)
Hi Ryan,

I've been away from FxOS for a long time now, so I'm not sure what
breaking changes might have landed, but I do have a guess.

(Also CCing :rpapa, who wrote this mock back in the day ;-)

Looking at the logs[1], around the 43 minute mark, I see this:

> 00:43:21     INFO -  /home/worker/gaia/apps/system/fxa/test/marionette/fxa_launch_test.js failed. Will retry.
> 00:44:15     INFO -  ....[marionette-mocha] Never saw webapps-registry-ready yes
> 00:44:18     INFO -  [marionette-mocha]
> 00:44:18     INFO -  [marionette-mocha] Error: listen EADDRINUSE
> 00:44:18     INFO -      at errnoException (net.js:905:11)
> 00:44:18     INFO -      at Server._listen2 (net.js:1043:14)
> 00:44:18     INFO -      at listen (net.js:1065:10)
> 00:44:18     INFO -      at Server.listen (net.js:1139:5)
> 00:44:18     INFO -      at Server.listen (/home/worker/gaia/node_modules/restify/lib/server.js:312:32)
> 00:44:18     INFO -      at Object.Server.start (/home/worker/gaia/apps/system/fxa/test/marionette/lib/server_mock_fxa.js:75:12)
> 00:44:18     INFO -      at /home/worker/gaia/apps/system/fxa/test/marionette/lib/server_mock_fxa.js:85:10
> 00:44:18     INFO -      at Object.<anonymous> (/home/worker/gaia/apps/system/fxa/test/marionette/lib/server_mock_fxa.js:87:3)
> 00:44:18     INFO -      at Module._compile (module.js:456:26)
> 00:44:18     INFO -      at Object.Module._extensions..js (module.js:474:10)
> 00:44:37     INFO -  ....[marionette-mocha]

It looks like the mock server is unable to (re)start, because the address
it uses is already in use. Either another mock is using the same address,
or the mock isn't releasing resources cleanly on the previous run.

Looking at the mock server[2], it is shutting down by calling process.disconnect.
I don't think this method is available on non-child processes, so it is
probably throwing when that line executes (if the mock ever runs as a non-
child process). I also don't see handlers for the `uncaughtException` event,
so if this error does occur, I'd guess the error is uncaught, so that the
mock server never quite exits. 

I might suggest using server.close instead to shut the mock down, as seen
in the restify tests[3]. Probably should also add `uncaughtException` listeners.

> // maybe something like this?
> 
> // use a cb if you need to orchestrate mocha shutdown
> function shutdown(cb) { 
>   try {
>     server.close(cb);
>   } catch (e) {
>     console.error(e.stack);
>     process.exit(1);
>   } 
> } 
> 
> function gracefulShutdown(err) {
>   if (err) { console.error(err.stack); }
>   shutdown();
> } 
> 
> process.on('uncaughtException', gracefulShutdown);
> server.on('uncaughtException', gracefulShutdown);

Docs for uncaughtException for restify are at [4] and for node, [5].
Note that the node docs say you should use domains for handling uncaught
errors at top level, but the consensus seems to be that domains are more
trouble than they're worth--see [6], for instance.

I suspect that not calling process.disconnect should fix one bug, and if
there are other unlogged exceptions happening in the mock, those will at
least be reported if this change is made.

:rpapa, sorry I missed this bug when the mock was first put together;
hopefully these suggestions are helpful and you have time to update
the mock. Feel free to ping me if you have questions.

[1] https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/Xv96vEBqQxa4GTrr7MpI1A/0/public/logs/live_backing.log
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/fxa/test/marionette/lib/server_mock_fxa.js
[3] https://github.com/restify/node-restify/blob/master/test/client.test.js#L203-L213
[4] http://restify.com/#event-uncaughtexception
[5] https://nodejs.org/api/process.html#process_event_uncaughtexception
[6] https://github.com/nodejs/io.js/issues/66
Flags: needinfo?(6a68) → needinfo?(rpappalardo)
Hey Jared,

Thanks for all the great notes!  Haven't looked at this since it landed, but I'm happy to take a look on Mon. (on PTO this week).
Working on a fix.  Probably have something by next week.
Flags: needinfo?(rpappalardo)
Attached image bug1189144.png
(In reply to Treeherder Robot from comment #6)
> log:
> https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-
> inbound&job_id=13203113
> repository: mozilla-inbound
> start_time: 2015-08-24T05:58:35
> who: rvandermeulen[at]mozilla[dot]com
> machine: unknown
> buildname: non-buildbot b2g-linux64 test [TC] - Gaia JS Integration Test
> revision: a7fb70a151a1
> 
> TypeError: Cannot call method 'stop' of undefined
> TEST-UNEXPECTED-FAIL | apps/system/fxa/test/marionette/fxa_launch_test.js |
> Firefox Accounts Launch Tests Should launch FxA flow from FxA-consuming
> apps: test-fxa-client app
> make: *** [test-integration-test] Error 1
> Return code: 2
> Tests exited with return code 2: harness failures
> # TBPL FAILURE #

This particular failure doesn't seem to be a failure with the testcase itself.  The screenshot shows webide boot screen 'based on Mozilla technology.'  not sure how the test would have gotten down to the teardown method if webide was still in the process of booting
We found a couple of issues with this test (and the other fxa_screen_flow_test).  Thanks to Jared's excellent notes we've improved the error checking / proc handling of the mock server with the fxa_screen_flow_test. This test (fxa_launch_test), however, doesn't need the mock server anyway, so we're removing the mock server call from it and that will fix this particular issue. It still fails on launching one of the older test clients, so we still need to look into that next.
Depends on: 1221390
No longer depends on: 1221390
See Also: → 1221390
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: