Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

.catch doesn't trigger for .remote #215

Open
mrkrstphr opened this issue Sep 26, 2018 · 7 comments
Open

.catch doesn't trigger for .remote #215

mrkrstphr opened this issue Sep 26, 2018 · 7 comments

Comments

@mrkrstphr
Copy link

I have an issue where I have a task to run database migrations via knex that is failing, but shipit just keeps running and never triggers the catch block.

  shipit.blTask('migrations', async () => {
    return shipit
      .remote(`npx knex migrate:latest`, {
        cwd: `${shipit.releasePath}/build`,
      })
      .catch(e => {
        console.log(e);
        process.exit();
      });
  });

If I console log inside .then() I see:

 [ { child: [ChildProcess],
       stdout:
        '\n> @ migrations:latest /var/www/api/releases/20180926144638/build\n> npx knex migrate:latest\n\nKnex:warning - migration file "20180926133945_TEST_THIS_WILL_FAIL.js" failed\nKnex:warning - migration failed with error: undefined\n',
       stderr:
        'Migration failed\nnpm ERR! code ELIFECYCLE\nnpm ERR! errno 1\nnpm ERR! @ migrations:latest: `npx knex migrate:latest`\nnpm ERR! Exit status 1\nnpm ERR! \nnpm ERR! Failed at the @ migrations:latest script.\nnpm ERR! This is probably not a problem with npm. There is likely additional logging output above.\n\nnpm ERR! A complete log of this run can be found in:\nnpm ERR!     /home/pemmy/.npm/_logs/2018-09-26T14_47_29_617Z-debug.log\n' } ] }
ChildProcess {
  _events:
   { close: [Function: exithandler],
     error: [Function: errorhandler] },
  _eventsCount: 2,
  _maxListeners: undefined,
  _closesNeeded: 3,
  _closesGot: 3,
  connected: false,
  signalCode: null,
  exitCode: 0,
...truncated

Not only does it not trigger the catch, but it shows an exit code of 0, which I assume is the exit code of the ssh command, not the command on the remote server.

The command on the remote server returns an exit code of 1.

How can I get this to trigger the .catch block so I can exit when migrations fail?

@utkarsh-dixit
Copy link

utkarsh-dixit commented Jul 9, 2019

If a callback function is provided, it is called with the arguments (error, stdout, stderr). On success, error will be null. On error, error will be an instance of Error. The error.code property will be the exit code of the child process while error.signal will be set to the signal that terminated the process. Any exit code other than 0 is considered to be an error.

The stdout and stderr arguments passed to the callback will contain the stdout and stderr output of the child process. By default, Node.js will decode the output as UTF-8 and pass strings to the callback. The encoding option can be used to specify the character encoding used to decode the stdout and stderr output. If encoding is 'buffer', or an unrecognized character encoding, Buffer objects will be passed to the callback instead.

@mrkrstphr As per child_process docs, the promise would be only rejected if there was some problem in the "child_process" module in not being able to start the command. If there is something wrong with the child process (in your case npx knex migrate:latest), it will return the error in as stderr. Inside the code make sure that, the stderr is empty string. Try the code below and it should work:-

  shipit.blTask('migrations', async () => {
    return shipit
      .remote(`npx knex migrate:latest`, {
        cwd: `${shipit.releasePath}/build`,
      }).then(result=>{
        if(result.stderr !== ""){
          console.log(result.stderr);
          process.exit();
        }
      });
  });

@jfilla
Copy link
Contributor

jfilla commented Aug 30, 2019

Same issue here. result.exitCode shoud have exit code from command, which is not happenning.

@rozsival
Copy link

rozsival commented Sep 1, 2019

Node child process is an event emitter, shipit-cli utils might allow to add custom event listeners on the process, or at least, add an error handler for exit event, which would cause a rejection of the promise returned, if child's exit code is not 0.

https://nodejs.org/docs/latest/api/child_process.html#child_process_event_exit

@jfilla
Copy link
Contributor

jfilla commented Dec 11, 2019

Any progress on this?

@jfilla
Copy link
Contributor

jfilla commented Mar 4, 2020

I found the problem. If you use cwd option, command is chained like this https://github.com/shipitjs/shipit/blob/master/packages/ssh-pool/src/commands/ssh.js#L4 and exit code is from cd command, which is zero if cwd exists.
@mrkrstphr quick fix for you is dont use cwd option and run cd by yourself.
cd ${cwd} && npx knex migrate:latest

@jfilla
Copy link
Contributor

jfilla commented Mar 4, 2020

PR #265

@BrockWills
Copy link

This is still occurring for me on the latest version of shipit (which looks to include the #265 PR mentioned above). It looks like this line here:

cd ${cwd} > /dev/null && ${command}; cd - > /dev/null

is not exiting even if command exits with a non-zero code, and then the cd - > /dev/null succeeds so that the final exit code is 0.

Changing it to just use && the whole way through like:

cd ${cwd} > /dev/null && ${command} && cd - > /dev/null

fixed it for me. Any specific reason it doesn't already work like that or should I open a PR for it?

Or even more specifically, since the .remote commands are each executed individually, and thus get their own shells, do we actually need the cd - > /dev/null command at the end since nothing is ever executed after it?

Note that for reference, the remote I'm running with is ubuntu which executes these commands via bash.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

5 participants