1040918 - utils.Commander does not error out if exception thrown

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Build, defect)

Description

[William Lachance (:wlach)]

We currently have the build system wired up to throw an exception if we can't successfully get a commit id using git:

https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/build/build.js#L110

However, this does not seem to be working. We're currently failing to do this, but the build just continues as normal:

E.g. https://tbpl.mozilla.org/php/getParsedLog.php?id=44126836&tree=B2g-Inbound&full=1 :

10:56:09     INFO -  [cmd] git --git-dir=/builds/slave/b2g_b2g-in_flame_eng_dep-00000/build/gaia/.git log -1 --format=%H%n%ct HEAD
10:56:09     INFO -  [settings-app-build] Error writing git commit file!
10:56:09     INFO -  stderr:
10:56:09     INFO -  undefined
10:56:09     INFO -  stdout: undefined
...
<build continues>

When we throw an exception, the build should fail noisily so we don't silently miss errors like this and accidentally ship slightly broken software. :)

Comment 1

[William Lachance (:wlach)]

Yuren, I'm going to assign this to you for now since you seem to understand the code in question the best.

Comment 2

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

it only happends for utils.Commander, I try to add "throw new Error('test')" on first line of |writeGitCommit()| and it works.

change title.

Comment 3

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

George, are you interested in this one?

Comment 4

[(Inactive after June) George Duan [:gduan] [:喬智]]

sure, take it first

Comment 5

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Alex,

I try to replace sh.run with sh.runWithSubprocess and put stderr inside the options,
(see https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/build/build.js#L99 and
     https://github.com/mozilla-b2g/gaia/blob/master/build/utils-xpc.js#L959)

I can dump the error message (utils.log) if I change 'git --git-dir=' to 'it --git-dir=' and the output is [/bin/sh: it: command not found]
. However, there's nothing happen if I use throw new Error(output).

Do you have any idea on it?

Comment 6

[Alexandre Poirot [:ochameau]]

I think you should assert the exitCode:
  done: function(result) {
    dump("process terminated with " + result.exitCode + "\n");
  },
It should be 0 if everything worked fine and >0 in case of error.

Also don't only test with a 'it --git-dir=', also test with a valid, but non working git command.
As /bin/sh seems to be involved here, you would need to verify that git error code is correctly forwarded by sh.

Comment 7

[(Inactive after June) George Duan [:gduan] [:喬智]]

It looks like we already throw error if commander goes wrong
https://github.com/mozilla-b2g/gaia/commit/e6cb523b9203f3d93c22280c6795f41bcc2dfa89#diff-2b31def028024b968babe1d78830dfafR954