pact-message return code ignored

[mikegeeves]

In message_pact.py we have:

def write_to_pact_file(self):
        """
        Create a pact file based on provided attributes in DSL.

        Return 0 if success, 1 otherwise.

        :rtype: int
        """
        command = [
            MESSAGE_PATH,
            "update",
            json.dumps(self._messages[0]),
            "--pact-dir", self.pact_dir,
            f"--pact-specification-version={self.version}",
            "--consumer", f"{self.consumer.name}",
            "--provider", f"{self.provider.name}",
        ]

        self._message_process = Popen(command)
        self._message_process.wait()

It claims to return 0 or 1 but does not!

Need to look at other implementations, how the STDERR/STDOUT is handled, this should maybe do something like Popen(command, stdout=PIPE, stderr=PIPE) and communicate instead of wait?

May simply be best to just "return self._message_process.wait()" -> but then how to handle later.

For reference, picked up while doing silly things with Term("5|15|60", 60) which is obviously invalid, causes ruby to fail, but it carries on regardless. The end result being if a previous pact file exists, it won't then fail. If one does not exist it fails because "No value provided for required pact_files" which isn't a helpful message.

[mefellows]

Pact JS (via the pact core) does this:

  public createMessage(): q.Promise<unknown> {
    logger.info(`Creating message pact`);
    const deferred = q.defer();
    const { pactFileWriteMode, content, ...restOptions } = this.options;

    const instance = spawn.spawnBinary(
      pactStandalone.messageFullPath,
      [{ pactFileWriteMode }, restOptions],
      this.__argMapping
    );
    const output: Array<string | Buffer> = [];
    instance.stdout.on('data', l => output.push(l));
    instance.stderr.on('data', l => output.push(l));
    instance.stdin.write(content);
    instance.once('close', code => {
      const o = output.join('\n');
      logger.info(o);

      if (code === 0) {
        return deferred.resolve(o);
      } else {
        return deferred.reject(o);
      }
    });

    instance.stdin.end();

    return deferred.promise;
  }

Essentially, it checks for an exit code of 0 and returns a fulfilled promise, else it returns a rejected promise. Additionally, it slurps the stderr/stdout from the command execution and prints it out at INFO level.

This would be equivalent to returning 0 or 1 I'd suggest.

[mikegeeves]

It's what happens next to the flow in the case of e.g. a rejected promise - it would be nice to have consistent messaging and make sure it's fatal etc.

Python is a little light on actual logging, that would be good to add in at some point.

I'd question INFO in this case (not that straightforward to just change because of the ordering etc), I'd expect WARN at the least if not ERROR if the end result is an overall error?

Will come back to this when there's a bit of time! :)

[mefellows]

In Pact JS's case, we return a rejected promise and it's up to the user to determine what to do with that information.

I'd question INFO in this case (not that straightforward to just change because of the ordering etc), I'd expect WARN at the least if not ERROR if the end result is an overall error?

Yeah, I believe there were historical reasons why we combined the two streams into a single output. I think the stream itself already contains a log level, so WARN should be visible in case of an error, but I'm not 100% certain on that.