publishPacts throws error even when publish was successful #758

Kampfmoehre · 2021-10-14T08:49:32Z

Software versions

OS: Fedora 34 (also Alpine in CI)
Consumer Pact library: @pact-foundation/pact 9.16.4 and @pact-foundation/pact-node 10.13.9
Provider Pact library: same as above
Node Version: v16.10.0

Issue Checklist

Please confirm the following:

I have upgraded to the latest
I have the read the FAQs in the Readme
I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures
I have set my log level to debug and attached a log file showing the complete request/response cycle
For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem

Expected behaviour

PactPublisher.publishPacts() does not throw an error

Actual behaviour

PactPublisher.publishPacts() throws an error

Steps to reproduce

We run our own Pact Broker and use a Let's Encrypt cert for it. We had problems with it since the old cert is expired and used PACT_DISABLE_SSL_VERIFICATION=true until I saw we can update the pact-node package in #754 . We publish using a JS script wrapping it in a try/catch block. Publish seems successful (the message is indicating it and I can see it in Pact Broker) but the call to publishPacts somehow throws an error.

Here is the code we use to publish pact:

/* eslint-disable no-console */
import type { PackageConfig } from "@company/internal-package";
import { Publisher } from "@pact-foundation/pact";
import type { PublisherOptions } from "@pact-foundation/pact-node";
import { resolve } from "path";

const publishOptions: Partial<PublisherOptions> = {};
const publishPact = async () => {
  try {
    if (process.env.PUBLISH_PACT !== "true") {
      console.log("Not publishing pacts since PUBLISH_PACT env variable is not set to true");
      return;
    }

    // eslint-disable-next-line @typescript-eslint/no-var-requires
    const packageJson = require(resolve(process.cwd(), "package.json")) as PackageConfig;
    if (!packageJson) {
      throw new Error("Unable to load package.json.");
    }

    publishOptions.consumerVersion = packageJson.version as string;

    if (!process.env.CI_COMMIT_SHORT_SHA) {
      console.warn(
        "No commit ref is set via CI_COMMIT_SHORT_SHA env variable, pact version will be package.json version only!",
      );
    } else {
      publishOptions.consumerVersion = `${publishOptions.consumerVersion}-${process.env.CI_COMMIT_SHORT_SHA}`;
    }
    if (process.env.CI_COMMIT_REF_NAME) {
      publishOptions.tags = [process.env.CI_COMMIT_REF_NAME];
    }

    publishOptions.pactBroker = process.env.PACT_BROKER_BASE_URL || "https://pact-broker.company.domain/";
    publishOptions.pactBrokerUsername = process.env.PACT_BROKER_USERNAME || "developer";
    publishOptions.pactBrokerPassword = process.env.PACT_BROKER_PASSWORD;
    publishOptions.pactFilesOrDirs = [resolve(process.cwd(), "pacts")];
    publishOptions.verbose = true;

    const result = await new Publisher(publishOptions as PublisherOptions).publishPacts();
    console.log("Successfully published pacts", result);
  } catch (err) {
    console.error("Unable to publish pacts:", err);
    process.exitCode = 1;
  }
};

void publishPact();

According to the stacktrace the error is thrown here: https://github.com/pact-foundation/pact-js-core/blob/v10.13.9/src/publisher.ts#L121

It looks like the error contains the whole message, as you can see in the logs below the output is logged and then when we log the error thrown by publishPacts() it is all logged again.

I have debugged a little bit and it seems that the regex is unable to extract the Pact Broker url. The string passed to it contains the whole message beginning with opening connection to pact-broker.company.domain:443... until No enabled webhooks found for the detected events. I am not sure if the regex is wrong because it expects to match the url from the beginning of the string or if the wrong string is pased to the regex.

Relevant log files

[2021-10-14 07:51:53.427 +0000] INFO (18884 on elitebook): pact-node@10.13.9: Publishing Pacts to Broker
[2021-10-14 07:51:53.429 +0000] INFO (18884 on elitebook): pact-node@10.13.9: Publishing pacts to broker at: https://pact-broker.company.domain/
[2021-10-14 07:51:55.225 +0000] ERROR (18884 on elitebook): pact-node@10.13.9: Could not publish pact:
opening connection to pact-broker.company.domain:443...
opened
starting SSL for pact-broker.company.domain:443...
SSL established
<- "GET /? HTTP/1.1\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nAccept: application/hal+json\r\nUser-Agent: Ruby\r\nAuthorization: [redacted]\r\n"
-> "HTTP/1.1 200 OK\r\n"
-> "Date: Thu, 14 Oct 2021 07:51:54 GMT\r\n"
-> "Content-Type: application/hal+json;charset=utf-8\r\n"
-> "Content-Length: 4613\r\n"
-> "Connection: keep-alive\r\n"
-> "Vary: Accept\r\n"
-> "X-Pact-Broker-Version: 2.86.0\r\n"
-> "X-Content-Type-Options: nosniff\r\n"
-> "Strict-Transport-Security: max-age=15724800; includeSubDomains\r\n"
-> "\r\n"
reading 4613 bytes...
-> "..."
read 4613 bytes
Conn keep-alive
opening connection to pact-broker.company.domain:443...
opened
starting SSL for pact-broker.company.domain:443...
SSL established
<- "POST /contracts/publish HTTP/1.1\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nAccept: application/hal+json\r\nUser-Agent: Ruby\r\nContent-Type: application/json\r\nAuthorization: [redacted]\r\n"
<- "..."

-> "HTTP/1.1 200 OK\r\n"
-> "Date: Thu, 14 Oct 2021 07:51:55 GMT\r\n"
-> "Content-Type: application/hal+json;charset=utf-8\r\n"
-> "Content-Length: 2943\r\n"
-> "Connection: keep-alive\r\n"
-> "Vary: Accept\r\n"
-> "X-Pact-Broker-Version: 2.86.0\r\n"
-> "X-Content-Type-Options: nosniff\r\n"
-> "Strict-Transport-Security: max-age=15724800; includeSubDomains\r\n"
-> "\r\n"
reading 2943 bytes...
-> "{\"logs\":[{\"level\":\"debug\",\"message\":\"Updated ConsumerPacticipant version 1.3.28-1afd334c with tags main\",\"deprecationWarning\":\"Replaced by notices\"},{\"level\":\"prompt\",\"message\":\"  Next steps:\\n    Configure the version branch to be the value of your repository branch.\",\"deprecationWarning\":\"Replaced by notices\"},{\"level\":\"success\",\"message\":\"Pact successfully republished for ConsumerPacticipant version 1.3.28-1afd334c and provider ProviderPacticipant with no content changes.\",\"deprecationWarning\":\"Replaced by notices\"},{\"level\":\"debug\",\"message\":\"  View the published pact at https://pact-broker.company.domain/pacts/provider/ProviderPacticipant/consumer/ConsumerPacticipant/version/1.3.28-1afd334c\",\"deprecationWarning\":\"Replaced by notices\"},{\"level\":\"debug\",\"message\":\"  Events detected: contract_published\",\"deprecationWarning\":\"Replaced by notices\"},{\"level\":\"debug\",\"message\":\"  No enabled webhooks found for the detected events\",\"deprecationWarning\":\"Replaced by notices\"}],\"notices\":[{\"type\":\"debug\",\"text\":\"Updated ConsumerPacticipant version 1.3.28-1afd334c with tags main\"},{\"type\":\"prompt\",\"text\":\"  Next steps:\\n    Configure the version branch to be the value of your repository branch.\"},{\"type\":\"success\",\"text\":\"Pact successfully republished for ConsumerPacticipant version 1.3.28-1afd334c and provider ProviderPacticipant with no content changes.\"},{\"type\":\"debug\",\"text\":\"  View the published pact at https://pact-broker.company.domain/pacts/provider/ProviderPacticipant/consumer/ConsumerPacticipant/version/1.3.28-1afd334c\"},{\"type\":\"debug\",\"text\":\"  Events detected: contract_published\"},{\"type\":\"debug\",\"text\":\"  No enabled webhooks found for the detected events\"}],\"_embedded\":{\"pacticipant\":{\"name\":\"ConsumerPacticipant\",\"_links\":{\"self\":{\"href\":\"https://pact-broker.company.domain/pacticipants/ConsumerPacticipant\"}}},\"version\":{\"number\":\"1.3.28-1afd334c\",\"_links\":{\"self\":{\"title\":\"Version\",\"name\":\"1.3.28-1afd334c\",\"href\":\"https://pact-broker.company.domain/pacticipants/ConsumerPacticipant/versions/1.3.28-1afd334c\"}}}},\"_links\":{\"pb:pacticipant\":{\"title\":\"Pacticipant\",\"name\":\"ConsumerPacticipant\",\"href\":\"https://pact-broker.company.domain/pacticipants/ConsumerPacticipant\"},\"pb:pacticipant-version\":{\"title\":\"Pacticipant version\",\"name\":\"1.3.28-1afd334c\",\"href\":\"https://pact-broker.company.domain/pacticipants/ConsumerPacticipant/versions/1.3.28-1afd334c\"},\"pb:pacticipant-version-tags\":[{\"title\":\"Tag\",\"name\":\"main\",\"href\":\"https://pact-broker.company.domain/pacticipants/ConsumerPacticipant/versions/1.3.28-1afd334c/tags/main\"}],\"pb:contracts\":[{\"title\":\"Pact\",\"name\":\"Pact between ConsumerPacticipant (1.3.28-1afd334c) and ProviderPacticipant\",\"href\":\"https://pact-broker.company.domain/pacts/provider/ProviderPacticipant/consumer/ConsumerPacticipant/version/1.3.28-1afd334c\"}]}}"
read 2943 bytes
Conn keep-alive
Updated ConsumerPacticipant version 1.3.28-1afd334c with tags main
  Next steps:
    Configure the version branch to be the value of your repository branch.
Pact successfully republished for ConsumerPacticipant version 1.3.28-1afd334c and provider ProviderPacticipant with no content changes.
  View the published pact at https://pact-broker.company.domain/pacts/provider/ProviderPacticipant/consumer/ConsumerPacticipant/version/1.3.28-1afd334c
  Events detected: contract_published
  No enabled webhooks found for the detected events

Unable to publish pacts: Error: opening connection to pact-broker.company.domain:443...
opened
starting SSL for pact-broker.company.domain:443...
SSL established
<- "GET /? HTTP/1.1\r\nAccept-Encoding: gzip;q=1.0,deflate;q=0.6,identity;q=0.3\r\nAccept: application/hal+json\r\nUser-Agent: Ruby\r\nAuthorization: [redacted]\r\n"
-> "HTTP/1.1 200 OK\r\n"
-> "Date: Thu, 14 Oct 2021 07:51:54 GMT\r\n"
-> "Content-Type: application/hal+json;charset=utf-8\r\n"
-> "Content-Length: 4613\r\n"
-> "Connection: keep-alive\r\n"
-> "Vary: Accept\r\n"
-> "X-Pact-Broker-Version: 2.86.0\r\n"
-> "X-Content-Type-Options: nosniff\r\n"
-> "Strict-Transport-Security: max-age=15724800; includeSubDomains\r\n"
-> "\r\n"
reading 4613 bytes...
-> "..."
read 4613 bytes
Conn keep-alive
opening connection to pact-broker.company.domain:443...
opened
starting SSL for pact-broker.company.domain:443...
SSL established
<- "..."

-> "HTTP/1.1 200 OK\r\n"
-> "Date: Thu, 14 Oct 2021 07:51:55 GMT\r\n"
-> "Content-Type: application/hal+json;charset=utf-8\r\n"
-> "Content-Length: 2943\r\n"
-> "Connection: keep-alive\r\n"
-> "Vary: Accept\r\n"
-> "X-Pact-Broker-Version: 2.86.0\r\n"
-> "X-Content-Type-Options: nosniff\r\n"
-> "Strict-Transport-Security: max-age=15724800; includeSubDomains\r\n"
-> "\r\n"
reading 2943 bytes...
-> "..."
read 2943 bytes
Conn keep-alive
Updated ConsumerPacticipant version 1.3.28-1afd334c with tags main
  Next steps:
    Configure the version branch to be the value of your repository branch.
Pact successfully republished for ConsumerPacticipant version 1.3.28-1afd334c and provider ProviderPacticipant with no content changes.
  View the published pact at https://pact-broker.company.domain/pacts/provider/ProviderPacticipant/consumer/ConsumerPacticipant/version/1.3.28-1afd334c
  Events detected: contract_published
  No enabled webhooks found for the detected events

    at ChildProcess.<anonymous> (/home/user/dev/project/node_modules/@pact-foundation/pact-node/src/publisher.ts:121:32)
    at Object.onceWrapper (node:events:510:26)
    at ChildProcess.emit (node:events:402:35)
    at ChildProcess.emit (node:domain:475:12)
    at maybeClose (node:internal/child_process:1064:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:301:5)

The text was updated successfully, but these errors were encountered:

TimothyJones · 2021-10-14T09:17:22Z

This is an excellent report! Thank you for all the detail. I should hopefully have some time to look into this this weekend.

bethesque · 2021-10-14T09:35:15Z

Is there a good reason for the pact url to be retuned in the promise? There could be multiple pacts published, so it's already buggy.

If you use --output json and parse the response from the CLI, you'll be able to get the pact URLs (plural) out. You'll need to replicate the behaviour of the CLI output in node, which prints the notices to stdout using the following colours: https://github.com/pact-foundation/pact_broker-client/blob/2a51e37/lib/pact_broker/client/colorize_notices.rb#L21

The documentation for the response returned by the new "all in one" publishing endpoint is here: https://github.com/pact-foundation/pact_broker/blob/master/lib/pact_broker/doc/views/index/publish-contracts.markdown

It will only use the "all in one" endpoint that has the fancy response notices if it has a new version of the broker though. For old brokers, it will publish the old way (separate calls for each tag and each pact) and it doesn't support --output json.

So you'd have to set --output json, and try parsing to to JSON, and if it is JSON, print the notices, and if isn't, fall back to the current behaviour.

TimothyJones · 2021-10-15T04:21:47Z

Is there a good reason for the pact url to be retuned in the promise?

Other than this is currently the interface, I don't think so. Pact-js doesn't actually even read the list, it just returns them to the user (which unfortunately means that changing it would be a breaking change).

There could be multiple pacts published, so it's already buggy.

I'm not sure I see the bug you're pointing out - my read of the current code is that it should work correctly if there are multiple URLs in the output (but also I can't see why this bug is happening either, so I'm clearly missing something).

If you use --output json and parse the response from the CLI,

Yes, I think we should definitely do this. Probably if we got the list of URLs from the JSON, it would also fix this bug.
(I believe this code pre-dates the ability to specify --json, which is why it's parsing the output)

You'll need to replicate the behaviour of the CLI output in node

Hmm. I'm not super keen to replicate the CLI - I don't think it'll provide much value, and it'll make work for pact-js every time the CLI changes. Pact-js exposes the CLI to npm scripts anyway - the publish part of the API is only for people building tools on top of pact.

I think a lot of people are still using their own custom publish script, because (until last week) the examples still used custom publish scripts (unnecessarily) instead of the CLI, even though that has been the easiest approach for a few years now - I don't think we highlighted the ability to use the CLI from npm scripts very well in the documentation.

@Kampfmoehre - do you have a particular use case for using the publishPacts function instead of the CLI - or is this a case of "the documentation didn't explain that I could do that"?

Kampfmoehre · 2021-10-15T08:14:33Z

Part of the reason is, we want to set up our version number correctly as well as the branch tag. This is integrated in our CI to run after Semantic Release, so we can't predict exactly what version the application has at this time. So we extract it from package.json (where Semantic-Release writes to). But there are other ways to do that (for example using jq on command line).

The other reason is, the script is some sort of legacy code that gets passed through every time we add a new project. I wanted to make this more common in our (GitLab) CI anyway so it works the same for .NET and NodeJS projects but haven't had the time so far.
I will take a look at the CLI, I assume there is a container image for that?

TimothyJones · 2021-10-15T08:23:06Z

You don't need a container image (although there is one) - pact-js exposes the CLI as bin stubs, so you can just use it direct in your npm script - see an example here:

    "pact:publish": "pact-broker publish pact/pacts --consumer-app-version=\"$(npx @pact-foundation/absolute-version)\" --tag-with-git-branch --broker-base-url=https://test.pact.dius.com.au"

(technically pact-node/pact-core bring in the CLI, but those are dependencies of pact-js, depending on which version you have. You shouldn't need to install them separately).

I'd be keen to hear if you get a good pattern going with semantic release - it has so many opinions on how your build process should run that I reckon it's worth publishing a pact plugin for it.

Kampfmoehre · 2021-10-15T08:42:23Z

I'm not sure if the pattern is good, but it works - for us at least. Since Pact is also part of the tests we run it two times. Of course we want to make sure tests pass before we make a release. For consumers this is not a problem since they work like unit tests and generate pact files. Provider tests don't offer that, so we run them in our test suite with publish = false. Both is just part of our test run - in CI as well as local.
If tests are successful pact files are passed to the following jobs in the CI pipeline. Then Semantic Release is run (typically building and pushing a new container image. From this job updated files are also passed to following CI jobs. This includes the modified package.json.
After that we publish pacts. Depending if the project has consumers or providers (many has both) two jobs are run. One just publishes the pact files via the script above, the other runs only the provider tests again with publish = true.
After that we run deploy jobs that use the Pact CLI with pact-broker can-i-deploy and pact-broker create-version-tag

Now a Semantic Release plugin would be relatively easy for consumer pacts (given an earlier stage provides the pact files), but for provider pacts it would need to know what tests to run and how to run them.

bethesque · 2021-10-15T10:24:04Z

I'm not sure I see the bug you're pointing out

I thought it returned a single URL, but you said "the list" Tim, so it must return multiple.

Hmm. I'm not super keen to replicate the CLI - I don't think it'll provide much value, and it'll make work for pact-js every time the CLI changes.

Very much agree.

mefellows · 2021-10-15T10:30:11Z

We definitely want to encourage direct use of the CLI and not the JS wrappers that were there for historical reasons. JS code can always exec the binary if needed, and there are many other ways to get the needed values into a CLI.

I think we should seriously be considering deprecating the existing pact-core APIs, because they all expose JS interfaces to the CLI which works against this principle.

I don't want this to become the next Maven/Gradle plugin problem!

bethesque · 2021-10-15T10:32:30Z

We should probably still fix the exit code issue though, for our existing users!

mefellows · 2021-10-15T10:35:49Z

Yes. We just got a separate issue that smells very similar.

TimothyJones · 2021-10-16T09:26:05Z

Oh, I see the bug. It's because the regex is expecting the pact url at the start of the line, and the output no longer does that.

To fix quickly, we can remove the ^ part of the regex.

To fix properly, I think we should:

Use --json and properly parse the output as Beth suggested
Not just throw all the output as the body of the error if we think there was an error (this meant that the unrelated permissions warning looked like it was the error) - can-i-deploy has a similar problem.

Other improvements:

Instead of emitting an error log for each line of the standard-error output, I propose we look for WARN or warning and emit warnings instead.

...also I have no idea how we put this bug into a release. This looks like it's tested to me, but again I must be missing something.

bethesque · 2021-10-17T19:48:40Z

To fix quickly, we can remove the ^ part of the regex.

Yes

To fix properly, I think we should:

I though we agreed above not to be trying to replicate the CLI output? Ideally, it should print the stdout it gets, and the stderr it gets, and pass on the exit code.

Plus, we want to be now discouraging its use in preference of using the CLI directly.

TimothyJones · 2021-10-18T10:19:56Z

This should start working again from pact-node v10.13.10

Kampfmoehre added the bug Indicates an unexpected problem or unintended behavior label Oct 14, 2021

Kampfmoehre mentioned this issue Oct 14, 2021

SSL certificate verify failed when publishing pacts #754

Closed

5 tasks

mefellows mentioned this issue Oct 15, 2021

Insecure world writable dir ... in PATH, mode 040777 pact-foundation/pact-js-core#338

Closed

TimothyJones closed this as completed Oct 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

publishPacts throws error even when publish was successful #758

publishPacts throws error even when publish was successful #758

Kampfmoehre commented Oct 14, 2021

TimothyJones commented Oct 14, 2021

bethesque commented Oct 14, 2021

TimothyJones commented Oct 15, 2021 •

edited

Kampfmoehre commented Oct 15, 2021

TimothyJones commented Oct 15, 2021

Kampfmoehre commented Oct 15, 2021

bethesque commented Oct 15, 2021

mefellows commented Oct 15, 2021

bethesque commented Oct 15, 2021

mefellows commented Oct 15, 2021

TimothyJones commented Oct 16, 2021 •

edited

bethesque commented Oct 17, 2021 •

edited

TimothyJones commented Oct 18, 2021

publishPacts throws error even when publish was successful #758

publishPacts throws error even when publish was successful #758

Comments

Kampfmoehre commented Oct 14, 2021

Software versions

Issue Checklist

Expected behaviour

Actual behaviour

Steps to reproduce

Relevant log files

TimothyJones commented Oct 14, 2021

bethesque commented Oct 14, 2021

TimothyJones commented Oct 15, 2021 • edited

Kampfmoehre commented Oct 15, 2021

TimothyJones commented Oct 15, 2021

Kampfmoehre commented Oct 15, 2021

bethesque commented Oct 15, 2021

mefellows commented Oct 15, 2021

bethesque commented Oct 15, 2021

mefellows commented Oct 15, 2021

TimothyJones commented Oct 16, 2021 • edited

bethesque commented Oct 17, 2021 • edited

TimothyJones commented Oct 18, 2021

TimothyJones commented Oct 15, 2021 •

edited

TimothyJones commented Oct 16, 2021 •

edited

bethesque commented Oct 17, 2021 •

edited