destroy should exit with non-zero code if --force was not provided but is necessary

[filmaj]

Describe the issue
Currently, if you have static assets in your arc app, and you run arc destroy, you will get a warning like this in your terminal:

⚬ Destroy Destroying staging environment
⚬ Destroy Reminder: if you deployed to production, don't forget to run destroy again with: --production
⚬ Destroy Destroying ConduitStagingPR5 immediately, hope you know what you're doing!
⚬ Destroy Destroying ConduitStagingPR5
⚠️ Warning: Found static bucket!
⚠️ Warning: To destroy this app (and any static assets and database tables that belong to it), run destroy with: --force

So, destroy does not end up executing, however, the exit code is 0, so things like CI systems end up completing successfully.

Expected behavior
destroy exits with a non-zero code in this situation.

[filmaj]

Related: seems like errors during destroy also exit with a zero status code.

[ryanblock]

Strongly agree with this! Something tells me @ryanbethel would agree, too!

[ryanbethel]

Yes I do agree. It is one of many things that can fail silently in CI. I my case I was able to use the heavy handed --force --now with destroy so I don't hit that particular problem. In other cases I had to test the output and manually run exit 1 to force the error.

[filmaj]

K, I'm gonna tackle this, and feed any errors into exiting with a non-zero code so destroy plays nice in CI!

[filmaj]

One additional point here: I think the current logic wrapping around CloudFormation deleteStack lacks the ability to handle a failed destroy. We specifically only look for a "missing stack" error, and nothing else.

The DestroyStack service reference is pretty simplistic, so not much can be gleaned from that call. We currently ping DescribeStack to glean the state of the Stack, and as mentioned, if a specific 'not found' error pops up as a response, then we know the stack was successfully removed. However, we don't handle the case where the delete could fail (i.e. DELETE_FAILED, see the StackStatus property on the DescribeStack response). So, I think I would like to add logic to destroy that looks for a Stack status of DELETE_FAILED, and exits with a non-zero code if that is detected. Additionally, we can glean the reason to a failure via the StackStatusReason property on the DescribeStack response.

WDYT?

[filmaj]

Well, adding an exit with a non-zero code on error was easy (architect/destroy@87fe6bf).

I'd still like to expand the logic as mentioned in my last comment to explicitly handle a failed delete better; current logic is such that if a delete fails early for whatever reason, the polling loop at the end of destroy won't catch it (since it's only looking for a "missing" aka deleted stack), and will spin until we hit the timeout.

[filmaj]

This was fixed in destroy 1.2.4 and published in architect 8.6.1.