fix: Handle uncaught exception in create-amplify

[vincetran]

Problem

Issue number, if available: #825

Changes

This change is similar to #2364 but the original change didn't fully fix the issue. Whilt it adds the error handler, it didn't change the try/catch loop and the error was being thrown by the getProjectRoot(). So modify the try block to surround all of the code that needs to close gracefully.

Corresponding docs PR, if applicable: N/A

Validation

CLI output:

❯ npx create-amplify
? Where should we create your project? (.)

╭─  ~/Development/backend/test-projects/test1 │ vincetran/crash-handling *
╰─

I triggered a SIGINT and the command stopped gracefully.

Checklist

• If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
• If this PR requires a change to the Project Architecture README, I have included that update in this PR.
• If this PR requires a docs update, I have linked to that docs PR above.
• If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[changeset-bot]

🦋 Changeset detected

Latest commit: b0f4a33

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
create-amplify	Patch
@aws-amplify/cli-core	Patch
@aws-amplify/backend-cli	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[sobolk]

some of these are incorrect if we're touching public API.

[vincetran]

Would this be more accurate?

'create-amplify': patch
'@aws-amplify/cli-core': minor
'@aws-amplify/backend-cli': minor

[sobolk]

'create-amplify': patch
'@aws-amplify/cli-core': minor
'@aws-amplify/backend-cli': patch

is the right mix.

[sobolk]

With this implementation we'll be blind to non-Errors.
We should handle them some way.
Can you please check how this is handled in other places?

[vincetran]

Forgive my ignorance as I'm new to the code base, but isn't that handled by the attachUnhandledExceptionListeners?

[sobolk]

No. once err is caught (by catch block) it's considered handled unless re-thrown.

You can assert this by adding throw 'foo' somewhere in try block and see what happens.

[vincetran]

Gotcha, I see that now thanks! I took a look around and it looks like largely, the downstream instances will wrap the non-Errors into an AmplifyError or some sort and then shoot it upstream. Meanwhile, in ampx, it only catches instances of Errors only.

So I could keep the original code that was here alongisde the handling of Error like so:

} catch (err) {
  if (err instanceof Error) {
    await errorHandler(format.error(err), err);
  } else {
    printer.log(format.error(err), LogLevel.ERROR);
    process.exitCode = 1;
  }
}

which shows up like so:

❯ npx create-amplify
2:51:22 PM [ERROR] foo

Thoughts?

[sobolk]

I do wonder if we can just remove the try-catch from here and just let it bubble up to "UnhandledExceptionListener" that we also introduce in this PR. It's implementation seems to be dealing with non-Errors.

[sobolk]

@Amplifiyer might have some ideas here as well.