Skip to content

Commit

Permalink
styra-link: unhandled bad download URL for styra CLI causes cryptic e…
Browse files Browse the repository at this point in the history
…rror (#78)

### What code changed, and why?

With a bad URL the automatic install of the Styra CLI by the Styra
VSCode plugin still resulted in storing _something_ at
/usr/local/bin/styra, but not an executable binary(!). 
The code assumed it was the real thing, reported
the CLI installation successful(!), tried to invoke it as the styra CLI,
and said _"Error spawn Unknown system error -8"_.

This PR will now recognize that the non-empty download
was not an executable, so fails the "install" operation properly.
  • Loading branch information
msorens authored Apr 17, 2024
1 parent 01a157e commit a8a9c0e
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 9 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- None
### Fixed

- Check that downloaded CLI binary is an actual binary (not a 404 page) instead of assuming it is.
With a bad download URL this caused a cryptic "Error spawn Unknown system error -8" message.

## [2.0.0] - 2023-09-25

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
[![slack](https://img.shields.io/badge/slack-styra-24b6e0.svg?logo=slack)](https://styracommunity.slack.com/)
[![Apache License](https://img.shields.io/badge/license-Apache%202.0-orange.svg)](https://www.apache.org/licenses/LICENSE-2.0)
[![Visual Studio Marketplace Version](https://img.shields.io/visual-studio-marketplace/v/Styra.vscode-styra?color=24b6e0)](#)
[![Coverage](https://img.shields.io/badge/Coverage-73%25-brightgreen)](#)
[![Coverage](https://img.shields.io/badge/Coverage-72%25-brightgreen)](#)
[![CI status](https://github.com/StyraInc/vscode-styra/actions/workflows/main.yaml/badge.svg)](https://github.com/StyraInc/vscode-styra/actions/workflows/main.yaml)
[![closed PRs](https://img.shields.io/github/issues-pr-closed-raw/StyraInc/vscode-styra)](https://github.com/StyraInc/vscode-styra/pulls?q=is%3Apr+is%3Aclosed)
<!--
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"url": "https://github.com/StyraInc/vscode-styra/issues"
},
"publisher": "styra",
"version": "2.0.1-next.1",
"version": "2.0.1-next.2",
"private": true,
"main": "./out/main.js",
"engines": {
Expand Down
1 change: 1 addition & 0 deletions src/commands/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export class Executor {
}
if (!await StyraInstall.styraCmdExists()) {
info(`"${STYRA_CLI_CMD}" not found; aborting ${command.title}`);
return;
}
// Deliberately delay the official start of a command until here;
// that way, any diagnostic output from the above pre-flight steps are nicely separated.
Expand Down
1 change: 0 additions & 1 deletion src/lib/command-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ export class CommandRunner {
const possibleError = options?.possibleError ?? '';
const quiet = options?.quiet ?? false;
if (!StyraInstall.checkWorkspace()) {
teeError('Something is wrong! Did you forget to run checkWorkspace in your command?');
return '';
}
const projectDir = IDE.projectDir();
Expand Down
10 changes: 8 additions & 2 deletions src/lib/styra-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,6 @@ export class StyraInstall {
}

private static async installStyra(): Promise<void> {
info(` Platform: ${process.platform}`);
info(` Architecture: ${process.arch}`);

const tempFileLocation = path.join(os.homedir(), this.BinaryFile);

Expand All @@ -167,6 +165,8 @@ export class StyraInstall {
cancellable: false
}, async () => {
await this.getBinary(url, tempFileLocation);
info(` Platform: ${process.platform}`);
info(` Architecture: ${process.arch}`);
info(` Executable: ${this.ExeFile}`);
fs.chmodSync(tempFileLocation, '755');
if (this.isWindows()) {
Expand All @@ -185,6 +185,12 @@ export class StyraInstall {
private static async getBinary(url: string, tempFileLocation: string): Promise<void> {
// adapted from https://stackoverflow.com/a/69290915
const response = await fetch(url);
if (!response.ok) {
throw new Error(
response.status === 404 ? `Bad URL for downloading styra - ${url}`
: `Error attempting to downloading styra CLI: status = ${response.status}`);
}

const writeStream = fse.createWriteStream(tempFileLocation, {
autoClose: true,
flags: 'w',
Expand Down
5 changes: 4 additions & 1 deletion src/test-jest/commands/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ describe('Executor', () => {
const spy = new OutputPaneSpy();
const spyAppendLine = jest.spyOn(outputChannel, 'appendLine');
Executor.checkStartup = jest.fn();
StyraInstall.styraCmdExists = jest.fn().mockResolvedValue(true);
IDE.getConfigValue = mockVSCodeSettings();

beforeEach(() => {
StyraInstall.styraCmdExists = jest.fn().mockResolvedValue(true);
});

[
[true, 'CONTINUES'],
[false, 'ABORTS'],
Expand Down

0 comments on commit a8a9c0e

Please sign in to comment.