Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLVSCODE-870 Fix silent failure of deployment to OpenVSX #628

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

jblievremont
Copy link
Member

@jblievremont jblievremont commented Oct 1, 2024

Copy link

sonarqube-next bot commented Oct 1, 2024

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@@ -29,10 +31,13 @@ const {
extensionFile: ARTIFACT_FILE,
pat: OPENVSX_TOKEN
};
await ovsx.publish(options);

const [ publicationResult ] = await ovsx.publish(options);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what caused the silent failure: ovsx.publish returns an array of PromiseSettledResult instead of failing 🤦🏻

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm looking at the correct thing, publish() returns a single Promise<PromiseSettledResult<void>[]>. With the await, we will get the array of settled/unsettled promises.

So I am thinking that we should consider the full array (not only the first element), and check if any of them have been rejected in the if below. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, yes; and when I run the script, the array contains only a single result. It could contain several items if we were publishing several packages at once.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that makes sense!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means all our release actions will start failing "loudly" from now on :D Let's discuss before we merge. Maybe it's best to disable the OVSX publish temporarily until we have a long-term solution

@jblievremont jblievremont merged commit 531d355 into master Oct 1, 2024
18 checks passed
@jblievremont jblievremont deleted the SLVSCODE-870-fix-ovsx-silent-failure branch October 1, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants