fix: replace searchAPI usage with GraphQL in findSRIssue util

[babblebey]

This Pull request replaces the usage of the troublesome 😉 GitHub SearchAPI as primary logic in the findSRIssue with graphql. It relegates the searchAPI consumption to a fallback logic that will be removed in a future release.

The GraphQL functionality performs its search for Semantic Release Issue by querying for Issues on a repository with specific filter, in our case we use a label in the filter. This means that the PR introduces changes to the way we "create" and "find" Sematic Release issues in the fail and success script.

Changes Made:

• Introduced an opinionated label for Semantic Release failing release issues in form of a constant RELEASE_FAIL_LABEL

• Concatenated the RELEASE_FAIL_LABEL to the label param on creation of semantic release failing issue in the fail script; this means the our opinionated label gets added to the failing release issue regardless of the plugin context.label configuration value (which is either semantic-release, CUSTOM_LABEL or false)

  const newIssue = {
    owner,
    repo,
    title: failTitle,
    body: `${body}\n\n${ISSUE_ID}`,
    labels: (labels || []).concat([RELEASE_FAIL_LABEL]), // 👈 👈 👈 👈  HERE
    assignees,
  };

• Introduced the GraphQL Query that the appropriated filter and passed into it is the labels filter with context.label configuration concatenated with our opinionated RELEASE_FAIL_LABEL as value;

  const {
    repository: {
      issues: { nodes: issueNodes },
    },
  } = await octokit.graphql(loadGetSRIssuesQuery, {
    owner,
    repo,
    filter: {
      labels: (labels || []).concat([RELEASE_FAIL_LABEL]), // 👈 👈 👈 👈  HERE
    },
  });

• Introduced error handling to the relegated GitHub searchAPI consumption; this allows for non-stop flow of step when an error occurs in the request to the hectic 😆 endpoint

• Converted (as initially said) GitHub searchAPI consumption to fallback logic which triggers when no fail issue is found through newly established means of finding fail issues

• Introduced the opinionated RELEASE_FAIL_LABEL to the failing release issues at creation as imperative label (no customizability)

Related Issue

Fixes #867

Screencast

DEMO: Closing all Opened Semantic Release Issues (with semantic-release label, RELEASE_FAIL_LABEL opinionated label and without any label)

screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.7.mp4

[babblebey]

BLOCKED HERE! HELP 🫨 📣

Everything works just as expected giving the PR a whirl.... But in the integration test coverage; the case below is bloody frustrating me 😭

github/test/integration.test.js

Lines 641 to 825 in 50c3f9c

	test("Verify, release and notify success", async (t) => {
	const owner = "test_user";
	const repo = "test_repo";
	const env = { GITHUB_TOKEN: "github_token" };
	const assets = [
	"upload.txt",
	{ path: "upload_other.txt", name: "other_file.txt", label: "Other File" },
	];
	const failTitle = "The automated release is failing 🚨";
	const options = {
	publish: [
	{ path: "@semantic-release/npm" },
	{ path: "@semantic-release/github", assets },
	],
	repositoryUrl: `https://github.com/${owner}/${repo}.git`,
	};
	const nextRelease = {
	gitTag: "v1.0.0",
	name: "v1.0.0",
	notes: "Test release note body",
	};
	const releaseUrl = `https://github.com/${owner}/${repo}/releases/${nextRelease.version}`;
	const assetUrl = `https://github.com/${owner}/${repo}/releases/download/${nextRelease.version}/upload.txt`;
	const otherAssetUrl = `https://github.com/${owner}/${repo}/releases/download/${nextRelease.version}/other_file.txt`;
	const releaseId = 1;
	const uploadOrigin = "https://github.com";
	const uploadUri = `/api/uploads/repos/${owner}/${repo}/releases/${releaseId}/assets`;
	const uploadUrl = `${uploadOrigin}${uploadUri}{?name,label}`;
	const prs = [{ number: 1, pull_request: {}, state: "closed" }];
	const commits = [{ hash: "123", message: "Commit 1 message" }];
	const fetch = fetchMock
	.sandbox()
	.get(
	`https://api.github.local/repos/${owner}/${repo}`,
	{
	permissions: { push: true },
	full_name: `${owner}/${repo}`,
	clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
	},
	{
	repeat: 2,
	},
	)
	.postOnce("https://api.github.local/graphql", {
	data: {
	repository: {
	commit123: {
	oid: "123",
	associatedPullRequests: {
	pageInfo: {
	endCursor: "NI",
	hasNextPage: false,
	},
	nodes: [prs[0]],
	},
	},
	},
	},
	})
	.postOnce(
	`https://api.github.local/repos/${owner}/${repo}/releases`,
	{
	upload_url: uploadUrl,
	html_url: releaseUrl,
	id: releaseId,
	},
	{
	body: {
	tag_name: nextRelease.gitTag,
	name: nextRelease.name,
	body: nextRelease.notes,
	draft: true,
	prerelease: false,
	},
	},
	)
	.patchOnce(
	`https://api.github.local/repos/${owner}/${repo}/releases/${releaseId}`,
	{ html_url: releaseUrl },
	{ body: { draft: false } },
	)
	.getOnce(
	`https://api.github.local/repos/${owner}/${repo}/pulls/1/commits`,
	[{ sha: commits[0].hash }],
	)
	.postOnce(
	`https://api.github.local/repos/${owner}/${repo}/issues/1/labels`,
	{},
	{ body: ["released"] },
	)
	.postOnce(
	(_, { body }) => {
	t.regex(JSON.parse(body).query, /query getSRIssues\(/);
	return true;
	},
	{
	data: {
	repository: {
	issues: { nodes: [] },
	},
	},
	},
	)
	.getOnce(
	`https://api.github.local/search/issues?q=${encodeURIComponent(
	"in:title",
	)}+${encodeURIComponent(`repo:${owner}/${repo}`)}+${encodeURIComponent(
	"type:issue",
	)}+${encodeURIComponent("state:open")}+${encodeURIComponent(failTitle)}`,
	{ items: [] },
	)
	.postOnce(
	`${uploadOrigin}${uploadUri}?name=${encodeURIComponent("upload.txt")}&`,
	{ browser_download_url: assetUrl },
	)
	.postOnce(
	`${uploadOrigin}${uploadUri}?name=other_file.txt&label=${encodeURIComponent(
	"Other File",
	)}`,
	{
	browser_download_url: otherAssetUrl,
	},
	)
	.postOnce(
	`https://api.github.local/repos/${owner}/${repo}/issues/1/comments`,
	{
	html_url: "https://github.com/successcomment-1",
	},
	);
	await t.notThrowsAsync(
	t.context.m.verifyConditions(
	{},
	{ cwd, env, options, logger: t.context.logger },
	{
	Octokit: TestOctokit.defaults((options) => ({
	...options,
	request: { ...options.request, fetch },
	})),
	},
	),
	);
	await t.context.m.publish(
	{ assets },
	{
	cwd,
	env,
	options,
	branch: { type: "release", main: true },
	nextRelease,
	logger: t.context.logger,
	},
	{
	Octokit: TestOctokit.defaults((options) => ({
	...options,
	request: { ...options.request, fetch },
	})),
	},
	);
	await t.context.m.success(
	{ assets, failTitle },
	{
	cwd,
	env,
	options,
	nextRelease,
	commits,
	releases: [],
	logger: t.context.logger,
	},
	{
	Octokit: TestOctokit.defaults((options) => ({
	...options,
	request: { ...options.request, fetch },
	})),
	},
	);
	t.deepEqual(t.context.log.args[0], ["Verify GitHub authentication"]);
	t.true(t.context.log.calledWith("Published file %s", otherAssetUrl));
	t.true(t.context.log.calledWith("Published file %s", assetUrl));
	t.true(t.context.log.calledWith("Published GitHub release: %s", releaseUrl));
	t.true(fetch.done());
	});

Here's what I've got:

In the context of this particular test case, I have only made 2 changes, and they're stated below...

• Introduced a graphql request mock to intercept the graphql request in findSRIssues; did this with a function matcher option ensuring no conflict with the existing graphql request mock for fetching associatedPRs

  .postOnce(
    // Matcher Starts
    (_, { body }) => {
      t.regex(JSON.parse(body).query, /query getSRIssues\(/);
      return true;
    },
    // Matcher Ends
    {
      data: {
        repository: {
          issues: { nodes: [] },
        },
      },
    },
  )

• THE SECOND CHANGE DYNAMICS: This change introduced different matcher to the associatedPRs graphql request mock and it leads to different errors based on the different matcher, state below...

  .postOnce(/MATCHER_HERE/, {
    data: {
      repository: {
        commit123: {
          oid: "123",
          associatedPullRequests: {
            pageInfo: {
              endCursor: "NI",
              hasNextPage: false,
            },
            nodes: [prs[0]],
          },
        },
      },
    },
  })

  • When matcher is plain old "https://api.github.local/graphql" (currently commited change); See current full error at https://github.com/semantic-release/github/actions/runs/10619388030/job/29436858061?pr=907#step:8:94 or see snippet below

    ```
    [test:integration]   HttpError (RequestError) {
    [test:integration]     cause: SyntaxError {
    [test:integration]       message: '"undefined" is not valid JSON',
    [test:integration]     },
    [test:integration]     request: {
    [test:integration]       body: {
    [test:integration]         body: 'Test release note body',
    [test:integration]         draft: true,
    [test:integration]         name: 'v1.0.0',
    [test:integration]         prerelease: false,
    [test:integration]         tag_name: 'v1.0.0',
    [test:integration]       },
    [test:integration]       headers: {
    [test:integration]         accept: 'application/vnd.github.v3+json',
    [test:integration]         authorization: 'token [REDACTED]',
    [test:integration]         'content-type': 'application/json; charset=utf-8',
    [test:integration]         'user-agent': 'test octokit-core.js/6.1.2 Node.js/20.9.0 (win32; x64)',
    [test:integration]       },
    [test:integration]       method: 'POST',
    [test:integration]       request: {
    [test:integration]         agent: undefined,
    [test:integration]         fetch: Function fetchMockProxy { … },
    [test:integration]         hook: Function bound bound register {},
    [test:integration]       },
    [test:integration]       url: 'https://api.github.local/repos/test_user/test_repo/releases',
    [test:integration]     },
    [test:integration]    
    ```
    

  • When matcher uses a function as seen below (this set-up similar to the matcher used in the mock request for findSRIssue uses digs into the request object an matches to intercepts request where query getAssociatedPRs string is present in the query property)

    (_, { body }) => {
      t.regex(JSON.parse(body).query, /query getAssociatedPRs\(/);
      return true;
    },

    Error:

    [test:integration]    685:     .postOnce((_, { body }) => {
    [test:integration]    686:       t.regex(JSON.parse(body).query, /query getAssociatedPRs\(/);
    [test:integration]    687:       return true;
    [test:integration]
    [test:integration]   `t.regex()` must be called with a string
    [test:integration]
    [test:integration]   Called with:
    [test:integration]
    [test:integration]   undefined
    

  • When matcher uses same function and asserts the request url, seen below

    (url, { body }) => {
      t.is(url, "https://api.github.local/graphql");
      t.regex(JSON.parse(body).query, /query getAssociatedPRs\(/);
      return true;
    }

    Error:

    [test:integration]    685:     .postOnce((url, { body }) => {
    [test:integration]    686:       t.is(url, "https://api.github.local/graphql");
    [test:integration]    687:       t.regex(JSON.parse(body).query, /query getAssociatedPRs\(/);
    [test:integration]
    [test:integration]   Difference (- actual, + expected):
    [test:integration]
    [test:integration]   - 'https://api.github.local/repos/test_user/test_repo/releases'
    [test:integration]   + 'https://api.github.local/graphql'
    

  • When matcher uses another supported function format, seen below

    (_, __, { body }) => {
      t.regex(JSON.parse(body).query, /query getAssociatedPRs\(/);
      return true;
    }

    Error:

    [test:integration]   HttpError (RequestError) {
    [test:integration]     cause: TypeError {
    [test:integration]       message: 'Cannot destructure property \'body\' of \'undefined\' as it is undefined.',
    [test:integration]     },
    [test:integration]     request: {
    [test:integration]       body: {
    [test:integration]         body: 'Test release note body',
    [test:integration]         draft: true,
    [test:integration]         name: 'v1.0.0',
    [test:integration]         prerelease: false,
    [test:integration]         tag_name: 'v1.0.0',
    [test:integration]       },
    [test:integration]       headers: {
    [test:integration]         accept: 'application/vnd.github.v3+json',
    [test:integration]         authorization: 'token [REDACTED]',
    [test:integration]         'content-type': 'application/json; charset=utf-8',
    [test:integration]         'user-agent': 'test octokit-core.js/6.1.2 Node.js/20.9.0 (win32; x64)',
    [test:integration]       },
    [test:integration]       method: 'POST',
    [test:integration]       request: {
    [test:integration]         agent: undefined,
    [test:integration]         fetch: Function fetchMockProxy { … },
    [test:integration]         hook: Function bound bound register {},
    [test:integration]       },
    [test:integration]       url: 'https://api.github.local/repos/test_user/test_repo/releases',
    [test:integration]     },
    [test:integration]     response: undefined,
    [test:integration]     status: 500,
    [test:integration]     message: 'Cannot destructure property \'body\' of \'undefined\' as it is undefined.',
    [test:integration]   }
    

Important Note

Delete the test case "Verify, release and notify success" itself 🫣 and every other 242 tests are passes.... I'm very convinced something is messed up in there that I need to figure out 😢

Cc @gr2m @travi

[babblebey]

BLOCKED HERE! HELP 🫨 📣

Everything works just as expected giving the PR a whirl.... But in the integration test coverage; the case below is bloody frustrating me 😭

Yoooooo, I addressed it 🥳🥳🥳🥳

What I did 🤌

Its important to note that in my function matchers, I implemented assertion as seen below....

(url, { body }) => {
  t.is(url, "https://api.github.local/graphql");
  t.regex(JSON.parse(body).query, /query getAssociatedPRs\(/);
  return true;
}

...well this works in some test cases and doesn't work in this particular troublesome case... But on a quick call with @gr2m today, I learnt I could/should just do away with assertions in the matchers... Hence I implement plain condition that return true when it finds a match for the mock... see below

(url, { body }) =>
  url === "https://api.github.local/graphql" &&
  JSON.parse(body).query.includes("query getAssociatedPRs("),

...and voila, the bad boy works 😁.

I was also experiencing similar troubles in #874 and all in now well 😉... I guess I can have a good weekend now 😃

[gr2m]

Let's gooo! We discussed this extensively and @babblebey tested it locally, time to ship 🚀

[github-actions]

🎉 This PR is included in version 10.3.5 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[zachspar]

@babblebey the issue no longer fails my pipeline 👍, but why is it still using GH SearchAPI at all as a fallback (it eats up time in pipeline)? Shouldn't the new mechanism of finding issues be sufficient? In my case, no issue was found using GraphQL, so it still went to old logic as fallback?

Thanks in advance!

  Request quota exhausted for request GET /search/issues
  Request quota exhausted for request GET /search/issues
  Request quota exhausted for request GET /search/issues
  Request quota exhausted for request GET /search/issues
  Request quota exhausted for request GET /search/issues
  2024-09-24T16:49:06.750Z semantic-release:github found semantic-release issues: []
  [4:49:06 PM] [semantic-release] › ℹ  An error occured fetching issue via fallback (with GH SearchAPI)

[babblebey]

Hey @zachspar,

Great question. The SearchAPI was used to search "semantic-release fail issue" using just the title, its a little different from how we are now searching with GraphQL which searches list of issues on a repo using labels... In order to enable empower this label search it meant that we had to change how we create the "semantic-release fail issue" in the first place, we did this by enforcing addition of an opinionated RELEASE_FAIL_LABEL which we will always filter with at search.

The need for SearchAPI has fallback arises in cases when their are "semantic-release fail issue" created by the old setup (these are issues that didn't get the opinionated RELEASE_FAIL_LABEL added at creation), so this fallback only triggers to ensure that these old issues are still caught if they exist.

[zachspar]

The need for SearchAPI has fallback arises in cases when their are "semantic-release fail issue" created by the old setup (these are issues that didn't get the opinionated RELEASE_FAIL_LABEL added at creation), so this fallback only triggers to ensure that these old issues are still caught if they exist.

Gotcha, so it is for backward compatibility. Will there be any plans to deprecate the fallback logic? Or is there a mechanism it could use to search for these issues using GraphQL without labels?

[babblebey]

Will there be any plans to deprecate the fallback logic?

Says in there 😉

github/lib/find-sr-issues.js

Lines 21 to 24 in b6ed55e

	/**
	* BACKWARD COMPATIBILITY: Fallback to the search API if the issue was not found in the GraphQL response.
	* This fallback will be removed in a future release
	*/

Or is there a mechanism it could use to search for these issues using GraphQL without labels?

None at the moment, but if we do find any GraphQL means that works as simply as we used the SearchAPI with title... that would be great and would mean we can completely deprecate usage of the SearchAPI.