semantic-release / commit-analyzer

:bulb: semantic-release plugin to analyze commits with conventional-changelog
MIT License
371 stars 74 forks source link

promisify crashing node #525

Closed SamMousa closed 1 year ago

SamMousa commented 1 year ago

I have several different packages where semantic release is no longer working. The last line in the log is this:

[3:27:11 PM] [semantic-release] › ℹ  Start step "analyzeCommits" of plugin "@semantic-release/commit-analyzer"

The exit code is 0 so my pipelines proceed but no release is created. This happens on several repositories, most open source, I'll link some CI traces here:

The code causing this issue lives in load-parser-config.js:

export default async ({ preset, config, parserOpts, presetConfig }, { cwd, logger }) => {
  let loadedConfig;
  const __dirname = dirname(fileURLToPath(import.meta.url));

  if (preset) {
    const presetPackage = `conventional-changelog-${preset.toLowerCase()}`;
    loadedConfig = importFrom(cwd, presetPackage);
  } else if (config) {
    loadedConfig = importFrom.silent(__dirname, config) || importFrom(cwd, config);
  } else {
    loadedConfig = conventionalChangelogAngular;
  }

  // Comments added by @sammousa. 
  // loadedConfig is an async function, specifically the createPreset export from conventional-changelog-conventionalcommits package
  // which implements the conventional commit standard that this package uses
  loadedConfig = await (typeof loadedConfig === "function"
      ? isPlainObject(presetConfig)
          ? loadedConfig(presetConfig)
          // this line crashes nodejs silently when called with an async function
          : promisify(loadedConfig)()
      : loadedConfig);

  return { ...loadedConfig.parserOpts, ...parserOpts };
};

The bug is definitely in nodeJS, the fix for this package however would be to not use promisify at all, since we're calling the function directly if there IS a configuration, why not also call it directly when there isn't?

travi commented 1 year ago

looks like you are running into a combination of semantic-release/semantic-release#2929 and semantic-release/semantic-release#2932. we have PRs started in https://github.com/semantic-release/release-notes-generator/pull/526 and https://github.com/semantic-release/commit-analyzer/pull/519 to address semantic-release/semantic-release#2929. in https://github.com/semantic-release/release-notes-generator/pull/526/files#diff-bee027b39eb704f3c940d54960f4f26693260c52d72707ac17d72f38f66da7d5L35-L40, we've removed that usage of promisify and plan to do the same in this package as well.

thanks for the investigation into the symptoms. i think we are covered this specific fix already in the beta, but interested in more feedback if you find more that we should investigate.

please give the beta version a try, as mentioned in semantic-release/semantic-release#2929, and let us know if you still have any problems with that.

SamMousa commented 1 year ago

For now my workaround is just to specify an empty presetConfig. I'm confused why i'm seeing exit code 0 though; since node itself should be dying with exit code 13...

travi commented 1 year ago

that is the reason we've created semantic-release/semantic-release#2932 we've had other reports of similar exiting with the wrong exit code for other failures. it is likely that we are not catching something properly somewhere. i think the key detail is that it is not specific to this issue, but is noticed when other legitimate failures occur. we agree that this is a problem, but have been mostly focused on semantic-release/semantic-release#2929 up to this point.

if you would be interested in helping us investigate this problem, we would greatly appreciate the help.

rvagg commented 1 year ago

It's because of this: https://github.com/conventional-changelog/conventional-changelog/pull/1045

conventional-changelog are now exporting async functions rather than functions that optionally accept a callback, so promisify() is silently failing, thinking it's done its work having not received anything back on the callback .. because there is none. So no errors, no exit code!=0.

This fixes it in lib/load-parser-config.js, but it's not pretty:

  loadedConfig = await (typeof loadedConfig === "function"
    ? isPlainObject(presetConfig)
      ? loadedConfig(presetConfig)
      : loadedConfig.constructor.name === 'AsyncFunction'
        ? loadedConfig()
        : promisify(loadedConfig)()
    : loadedConfig);

For me, the workaround is to pin to conventional-changelog-conventionalcommits@6 because 7.0.0 is where the change is introduced: https://github.com/conventional-changelog/conventional-changelog/releases/tag/conventional-changelog-conventionalcommits-v7.0.0

My bad for not pinning versions properly in my hacky Github Actions config.

travi commented 1 year ago

It's because of this: conventional-changelog/conventional-changelog#1045

thanks for the additional investigation. good to hear that this particular situation is likely to be covered by our pending changes.

we're working on getting a beta version promoted to stable to handle the updates from conventional-changelog, which should handle this and remove the use of promisify. that beta can be found at https://github.com/semantic-release/semantic-release/pull/2934. once we get that out, we still plan to investigate improper exits since we have had some other reports before this conventional-changelog update, so i think we still have some additional situations to handle.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 11.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: