-
Notifications
You must be signed in to change notification settings - Fork 83
fix: throw precise, actionable error when downloaded deno cli is unusable #6708
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
Changes from all commits
3f7574a
9b4c3a2
90aeab2
178241d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,14 +84,24 @@ export class DenoBridge { | |
| this.logger.system(`Downloading Deno CLI to ${this.cacheDirectory}`) | ||
|
|
||
| const binaryPath = await download(this.cacheDirectory, this.versionRange, this.logger) | ||
| const downloadedVersion = await this.getBinaryVersion(binaryPath) | ||
| const result = await this.getBinaryVersion(binaryPath) | ||
|
|
||
| // We should never get here, because it means that `DENO_VERSION_RANGE` is | ||
| // a malformed semver range. If this does happen, let's throw an error so | ||
| // that the tests catch it. | ||
| if (downloadedVersion === undefined) { | ||
| // If we can't execute the downloaded binary, provide actionable info to diagnose and self-heal if possible | ||
| if (result.error) { | ||
| const error = new Error( | ||
| 'There was a problem setting up the Edge Functions environment. To try a manual installation, visit https://ntl.fyi/install-deno.', | ||
| `Failed to set up Deno for Edge Functions. | ||
| Error: ${result.error.message} | ||
| Downloaded to: ${binaryPath} | ||
| Platform: ${process.platform}/${process.arch} | ||
|
|
||
| This may be caused by permissions, antivirus software, or platform incompatibility. | ||
|
|
||
| Try clearing the Deno cache directory and retrying: | ||
| ${this.cacheDirectory} | ||
|
Comment on lines
+99
to
+100
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided not to bother dynamically producing a platform-specific command here. I think users can figure out how to delete a directory. |
||
|
|
||
| Supported Deno versions: ${this.versionRange} | ||
|
|
||
| To install Deno manually: https://ntl.fyi/install-deno`, | ||
| ) | ||
|
|
||
| await this.onAfterDownload?.(error) | ||
|
|
@@ -101,26 +111,29 @@ export class DenoBridge { | |
| throw error | ||
| } | ||
|
|
||
| await this.writeVersionFile(downloadedVersion) | ||
| await this.writeVersionFile(result.version) | ||
|
|
||
| await this.onAfterDownload?.() | ||
|
|
||
| return binaryPath | ||
| } | ||
|
|
||
| async getBinaryVersion(binaryPath: string) { | ||
| async getBinaryVersion( | ||
| binaryPath: string, | ||
| ): Promise<{ version: string; error?: undefined } | { version?: undefined; error: Error }> { | ||
| try { | ||
| const { stdout } = await execa(binaryPath, ['--version']) | ||
| const version = stdout.match(/^deno ([\d.]+)/) | ||
|
|
||
| if (!version) { | ||
| this.logger.system(`getBinaryVersion no version found. binaryPath ${binaryPath}`) | ||
| return | ||
| return { error: new Error('Could not parse Deno version from output') } | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we include the full |
||
| } | ||
|
|
||
| return version[1] | ||
| return { version: version[1] } | ||
| } catch (error) { | ||
| this.logger.system('getBinaryVersion failed', error) | ||
| return { error: error instanceof Error ? error : new Error(String(error)) } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -152,11 +165,11 @@ export class DenoBridge { | |
| } | ||
|
|
||
| const globalBinaryName = 'deno' | ||
| const globalVersion = await this.getBinaryVersion(globalBinaryName) | ||
| const result = await this.getBinaryVersion(globalBinaryName) | ||
|
|
||
| if (globalVersion === undefined || !semver.satisfies(globalVersion, this.versionRange)) { | ||
| if (result.error || !semver.satisfies(result.version, this.versionRange)) { | ||
| this.logger.system( | ||
| `No globalVersion or semver not satisfied. globalVersion: ${globalVersion}, versionRange: ${this.versionRange}`, | ||
| `No globalVersion or semver not satisfied. globalVersion: ${result.version}, versionRange: ${this.versionRange}`, | ||
| ) | ||
| return | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line could be better 🤔