Skip to content

Commit

Permalink
Merge pull request #2274 from Shopify/improve-cloudlfare-error-stable
Browse files Browse the repository at this point in the history
Improve Cloudflare errors (on stable/3.47)
  • Loading branch information
gonzaloriestra authored Jun 30, 2023
2 parents 75ff0aa + af354a7 commit 4f38847
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 13 deletions.
7 changes: 7 additions & 0 deletions .changeset/short-yaks-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@shopify/plugin-cloudflare': patch
'@shopify/cli-kit': patch
'@shopify/app': patch
---

Improve Cloudflare errors
2 changes: 1 addition & 1 deletion packages/app/oclif.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@
"name": "tunnel",
"type": "option",
"description": "Select the tunnel provider",
"hidden": true,
"hidden": false,
"multiple": false,
"options": [
"cloudflare",
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/commands/app/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default class Dev extends Command {
exclusive: ['tunnel-url', 'tunnel'],
}),
tunnel: Flags.string({
hidden: true,
hidden: false,
description: 'Select the tunnel provider',
env: 'SHOPIFY_FLAG_TUNNEL',
default: 'cloudflare',
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-kit/src/public/node/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class AbortSilentError extends FatalError {
* A bug error is an error that represents a bug and therefore should be reported.
*/
export class BugError extends FatalError {
constructor(message: OutputMessage, tryMessage: TokenItem | null = null) {
constructor(message: TokenItem | OutputMessage, tryMessage: TokenItem | OutputMessage | null = null) {
super(message, FatalErrorType.Bug, tryMessage)
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/cli-kit/src/public/node/plugins/tunnel.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {ExtendableError} from '../error.js'
import {OutputMessage} from '../output.js'
import {FanoutHookFunction, PluginReturnsForHook} from '../plugins.js'
import {err, Result} from '../result.js'
import {TokenItem} from '../ui.js'

export type TunnelErrorType = 'invalid-provider' | 'tunnel-already-running' | 'wrong-credentials' | 'unknown'
export interface TunnelClient {
Expand All @@ -13,7 +15,7 @@ export type TunnelStatusType =
| {status: 'not-started'}
| {status: 'starting'}
| {status: 'connected'; url: string}
| {status: 'error'; message: string; tryMessage?: string}
| {status: 'error'; message: TokenItem | OutputMessage; tryMessage?: TokenItem | OutputMessage | null}

export class TunnelError extends ExtendableError {
type: TunnelErrorType
Expand Down
6 changes: 5 additions & 1 deletion packages/plugin-cloudflare/src/tunnel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ describe('hookStart', () => {

// Then
expect(exec).toBeCalledTimes(5)
expect(result).toEqual({status: 'error', message: 'Could not start tunnel, max retries reached'})
expect(result).toEqual({
status: 'error',
message: 'Could not start Cloudflare tunnel, max retries reached.',
tryMessage: expect.anything(),
})
})
})
25 changes: 17 additions & 8 deletions packages/plugin-cloudflare/src/tunnel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {AbortController} from '@shopify/cli-kit/node/abort'
import {joinPath, dirname} from '@shopify/cli-kit/node/path'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
import {AbortError} from '@shopify/cli-kit/node/error'
import {BugError} from '@shopify/cli-kit/node/error'
import {Writable} from 'stream'
import {fileURLToPath} from 'url'

Expand Down Expand Up @@ -62,7 +62,11 @@ class TunnelClientInstance implements TunnelClient {

if (retries >= MAX_RETRIES) {
resolved = true
this.currentStatus = {status: 'error', message: 'Could not start tunnel, max retries reached'}
this.currentStatus = {
status: 'error',
message: 'Could not start Cloudflare tunnel, max retries reached.',
tryMessage: whatToTry(),
}
return
}

Expand Down Expand Up @@ -122,9 +126,9 @@ class TunnelClientInstance implements TunnelClient {
}
// If already resolved, means that the CLI already received the tunnel URL.
// Can't retry because the CLI is running with an invalid URL
if (resolved) throw processCrashed()
if (resolved) throw processCrashed(error.message)

outputDebug('Cloudflared tunnel crashed, restarting...')
outputDebug(`Cloudflared tunnel crashed: ${error.message}, restarting...`)

// wait 1 second before restarting the tunnel, to avoid rate limiting
if (!isUnitTest()) await sleep(1)
Expand All @@ -134,18 +138,23 @@ class TunnelClientInstance implements TunnelClient {
}
}

function processCrashed() {
return new AbortError(`Tunnel process crashed after stablishing a connection.`, [
function processCrashed(message: string) {
return new BugError(`Tunnel process crashed after stablishing a connection: ${message}`, whatToTry())
}

function whatToTry() {
return [
'What to try:',
{
list: {
items: [
['Try to run the command again'],
['Run the command again'],
['Add the flag', {command: '--tunnel-url {URL}'}, 'to use a custom tunnel URL'],
['Add the flag', {command: '--tunnel ngrok'}, 'to use Ngrok as the tunnel provider'],
],
},
},
])
]
}

function findUrl(data: Buffer): string | undefined {
Expand Down

0 comments on commit 4f38847

Please sign in to comment.