Curious behavior when running programmatically

[StevenLangbroek]

Hey folks! I wrote a little script for normal semantic-release, which does a dry run, and then creates (or updates) a PR on Github with preliminary release notes and a next version.

I thought I'd port this to multi-semantic-release, but I'm running into a weird issue now. multi-semantic-release seems to be swallowing all logs after I run await multiSemanticRelease(pkgs, options), and exit with status 0. I'm not sure where to even start debugging tbh. I've added log statements to multi-semantic-release in node_modules, for example:

await Promise.all(packages.map(async (pkg) => {
  // snipped for brevity
}));
console.log('postRelease');

But even that postRelease never actually reaches my terminal. Any idea what's going on here?

[StevenLangbroek]

Also maybe of interest, if I change the tail end of the releasePackage function like so:

    // Call semanticRelease() on the directory and save result to pkg.
	// Don't need to log out errors as semantic-release already does that.
	const result = await semanticRelease(options, {
		cwd: dir,
		env,
		stdout: new RescopedStream(stdout, name),
		stderr: new RescopedStream(stderr, name),
	});

	console.log(result)

	pkg.result = result;

this result actually does get logged... super weird...

[StevenLangbroek]

I think what happens is, if not all packages have releases, semantic-release exits on the package that doesn't have a release? I'm still digging...

[antongolub]

The way we determine whether a package has been released is definitely in need of improvement.

[StevenLangbroek]

@antongolub should I consider using multi-semantic-release in this way as not working right now while we figure this out? If you want I can show you the behavior. The repo is private but I'd gladly jump on a call or some such and walk you through it.

[antongolub]

@StevenLangbroek

If you want I can show you the behavior.

Yes, please. Could you clarify the general purpose? It is always best to have a repository that illustrates the problem.

[StevenLangbroek]

Sure! So, we use CircleCI. What I'd like is every commit on develop to trigger a job, that runs semantic-release in dryRun, capture the results, then using @octokit/rest create a pull request to stable (our release branch), or if there already is one, update it with the new release notes. The code goes roughly as follows:

  const packages = pkg.workspaces.packages.map(package => {
    return path.join(process.cwd(), package, 'package.json')
  });
  
  try {
    const options = {
      dryRun: true,
      plugins: [
        '@semantic-release/commit-analyzer',
        '@semantic-release/release-notes-generator',
      ],
      branches: [CIRCLE_BRANCH],
      ci: true,
    };
    const releaseResults = await semanticRelease(packages, options)
    
    const notes = releaseResults.reduce(combine, '# Release Notes');

    createOrUpdatePR(notes, releaseResults);
    
  } catch (err) {
    console.error(err);
    process.exit(1);
  }

It worked when I was initially testing it locally, but when I actually merged my work into develop, I noticed no PR was created. I added some console statements, e.g., right after await semanticRelease(), but the code is never reached somehow (despite the notes being written to stdout, the code after is just never executed).

I also tried monkey-patching process.exit to figure out what's calling exit, but it would appear there's just a return statement short-circuiting something (or some such). My monkey-patched process.exit was just never called.

What I then tried is what I described in my initial comment; it seems logging before multi-semantic-release maps over packages and calls semanticRelease on them works, but everything after is also just not reached.

[StevenLangbroek]

Could it be that semantic-release is internally stateful in some ways that in edge-cases break parallelism?

[antongolub]

@StevenLangbroek

I say carefully: anything is possible. semrel architecture is based on the fact that single one thread controls the state of the repository during release. Parallel flow can break the logic of these operations in some cases.

NB A short-term break is scheduled due to vacations. I may be slow to response.

[StevenLangbroek]

No problem at all. Could we work around this by spawning perhaps?

[StevenLangbroek]

Or would that not really solve anything? 🤔

[StevenLangbroek]

Anyway! Enjoy your vacation 🍹 🌴 , let me know when you're back and we can maybe continue investigating.

[antongolub]

Deal.

[StevenLangbroek]

Hey @antongolub :). Hope you had a great vacation 🥳 . Can we continue figuring out what's going on here maybe?

[antongolub]

Hi, @StevenLangbroek,

I'll take another look soon. Could you provide a ref to the code of your experiments?

[ThisIsMissEm]

I've been picking this up tonight, and it's very strange. I think it's a bug in the synchroniser, but I'm not entirely sure. I've been messing around with other ways of awaiting for all the semantic releases to complete, and I'm noticing that only one package ever successfully completes, and once that package is complete, then entire script exits because there's nothing left to do (no more items on the event loop)

That's why the exit is a 0 status code; To node, as soon as the event loop is empty, it'll exit with 0

From what I can tell there is a bug somewhere in the synchroniser or promise workflow in multiSemanticRelease which results in these lines never being reached because the event loop empties out: https://github.com/dhoulb/multi-semantic-release/blob/master/lib/multiSemanticRelease.js#L92-L96

[ThisIsMissEm]

Could there possibly be a bug in the underlying library used in the synchroniser? I note that it's at 0.2.0, though it may have 92% test coverage. I just can't explain at all how the code is executing the way it does..

[antongolub]

Hey, @ThisIsMissEm,

Is the problem only affecting JS API? Could you run msr with debug flag? This may help to clarify a bit more details.

	// Status sync point.
	const waitForAll = (probe, filter = identity) => {
		const promise = once(probe);

		if (
			todo()
				.filter(filter)
				.every((p) => p.hasOwnProperty(probe))
		) {
			debug("ready: %s", probe);
			emit(probe);
		}

		return promise;
	};

...
	const emit = (probe, pkg) => {
		const name = getEventName(probe, pkg);
		debug("ready: %s", name);

		return store.evt[name] || (store.evt[name] = ee.emit(name));
	};

[ThisIsMissEm]

Hey @antongolub, I've ended up taking a step back, and I'm now looking into the viability of adopting Batching Toposort, which is explicitly designed for scheduling work like what we are trying to achieve with the synchronizer. I'll follow up with some more information soon.

As far as I understand it, the goal of the synchonizer is to execute all releases in layers, so we don't do the release of one package before it's dependencies are released. I don't think there's an explicit need to synchronise individual steps, as we should just have a DAG of packages (a cyclic graph of packages could never be successfully released, as there'd always be one package that's out of date).

Once we take that DAG and break it down into layers to execute, then there shouldn't really be any reason why we can't just let semantic-release go do it's thing.

[ThisIsMissEm]

@antongolub okay, so, I've had some initial good results with using batching toposort; The tests are broken right now, but I don't think they should've worked in the first place, as they featured a cyclic dependency chain: #46