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

Improve getting started experience #7011

Merged
merged 5 commits into from May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 41 additions & 4 deletions src/apphosting/index.ts
Expand Up @@ -2,7 +2,7 @@
import * as poller from "../operation-poller";
import * as apphosting from "../gcp/apphosting";
import * as githubConnections from "./githubConnections";
import { logBullet, logSuccess, logWarning } from "../utils";
import { logBullet, logSuccess, logWarning, sleep } from "../utils";
import {
apphostingOrigin,
artifactRegistryDomain,
Expand Down Expand Up @@ -36,6 +36,33 @@
maxBackoff: 10_000,
};

async function tlsReady(url: string): Promise<boolean> {
// Note, we do not use the helper libraries because they impose additional logic on content type and parsing.
try {
await fetch(url);
return true;
} catch (err) {
// At the time of this writing, the error code is ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE.
// I've chosen to use a regexp in an attempt to be forwards compatible with new versions of
// SSL.
const maybeNodeError = err as { cause: { code: string } };
if (/HANDSHAKE_FAILURE/.test(maybeNodeError?.cause?.code)) {

Check warning on line 49 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Use `String#includes()` method with a string instead
return false;
}
return true;
}
}

async function awaitTlsReady(url: string): Promise<void> {
let ready;
do {
ready = await tlsReady(url);
if (!ready) {
await sleep(1000 /* ms */);
}
} while (!ready);
}

/**
* Set up a new App Hosting backend.
*/
Expand Down Expand Up @@ -121,6 +148,7 @@
});

await setDefaultTrafficPolicy(projectId, location, backendId, branch);
logSuccess(`Successfully created backend:\n\t${backend.name}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove, this is already on L138

Copy link
Member Author

Choose a reason for hiding this comment

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

ugh. merge issues. I had previously moved and now it got turned into an add. TY for the catch.


const confirmRollout = await promptOnce({
type: "confirm",
Expand All @@ -134,11 +162,14 @@
return;
}

const url = `https://${backend.uri}`;
logBullet(
`You may also track this rollout at:\n\t${consoleOrigin()}/project/${projectId}/apphosting`,
);
// TODO: Previous versions of this command printed the URL before the rollout started so that
// if a user does exit they will know where to go later. Should this be re-added?
const createRolloutSpinner = ora(
"Starting a new rollout... This make take a few minutes. It's safe to exit now.",
"Starting a new rollout; this make take a few minutes. It's safe to exit now.",
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: this may take

).start();
await orchestrateRollout(projectId, location, backendId, {
source: {
Expand All @@ -147,7 +178,13 @@
},
},
});
createRolloutSpinner.succeed(`Your backend is now deployed at:\n\thttps://${backend.uri}`);
createRolloutSpinner.succeed("Rollout complete");
if (!(await tlsReady(url))) {
const tlsSpinner = ora("Waiting for TLS certificate");
Copy link
Contributor

Choose a reason for hiding this comment

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

you probably want tlsSpinner = ora(...).start()

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops! Maybe Ora isn't as easy to use as I thought!

Copy link
Contributor

Choose a reason for hiding this comment

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

"Finalizing your backend's TLS certificate; this may take a few minutes."

await awaitTlsReady(url);
tlsSpinner.succeed("TLS certificate ready");
}
logSuccess(`Your backend is now deployed at:\n\thttps://${backend.uri}`);
}

/**
Expand Down Expand Up @@ -191,19 +228,19 @@
async function promptNewBackendId(
projectId: string,
location: string,
prompt: any,

Check warning on line 231 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
): Promise<string> {
while (true) {

Check warning on line 233 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected constant condition
const backendId = await promptOnce(prompt);

Check warning on line 234 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe argument of type `any` assigned to a parameter of type `QuestionsThatReturnAString<Answers>`
try {
await apphosting.getBackend(projectId, location, backendId);
} catch (err: any) {

Check warning on line 237 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
if (err.status === 404) {

Check warning on line 238 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
return backendId;
}
throw new FirebaseError(
`Failed to check if backend with id ${backendId} already exists in ${location}`,
{ original: err },

Check warning on line 243 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
);
}
logWarning(`Backend with id ${backendId} already exists in ${location}`);
Expand Down Expand Up @@ -259,9 +296,9 @@
"Default service account used to run builds and deploys for Firebase App Hosting",
"Firebase App Hosting compute service account",
);
} catch (err: any) {

Check warning on line 299 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
// 409 Already Exists errors can safely be ignored.
if (err.status !== 409) {

Check warning on line 301 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .status on an `any` value
throw err;
}
}
Expand Down Expand Up @@ -341,7 +378,7 @@
if (tries >= 5) {
throw err;
}
await new Promise((resolve) => setTimeout(resolve, 1000));
await sleep(1000);
} else {
throw err;
}
Expand Down Expand Up @@ -405,7 +442,7 @@
*/
export async function promptLocation(
projectId: string,
prompt: string = "Please select a location:",

Check warning on line 445 in src/apphosting/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Type string trivially inferred from a string literal, remove type annotation
): Promise<string> {
const allowedLocations = (await apphosting.listLocations(projectId)).map((loc) => loc.locationId);
if (allowedLocations.length === 1) {
Expand Down
5 changes: 5 additions & 0 deletions src/utils.ts
Expand Up @@ -542,6 +542,11 @@ export async function promiseWithSpinner<T>(action: () => Promise<T>, message: s
return data;
}

/** Creates a promise that resolves after a given timeout. await to "sleep". */
export function sleep(ms: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, ms));
}

/**
* Return a "destroy" function for a Node.js HTTP server. MUST be called on
* server creation (e.g. right after `.listen`), BEFORE any connections.
Expand Down