[Resolve #1519] Improve error message

[dboitnot]

Resolves #1519 by:

• Raising un-handled exceptions which (I believe) should be the default behavior.
• Detecting ForbiddenException and wrapping it in a more informative message

PR Checklist

• Wrote a good commit message & description [see guide below].
• Commit message starts with [Resolve #issue-number].
• Added/Updated unit tests.
• Added/Updated integration tests (if applicable).
• All unit tests (poetry run tox) are passing.
• Used the same coding conventions as the rest of the project.
• The new code passes pre-commit validations (poetry run pre-commit run --all-files).
• The PR relates to only one subject with a clear title.
  and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

• Before merge squash related commits.

Other Information

Guide to writing a good commit

[alex-harvey-z3q]

In general I am a bit concerned that while I support the general idea, the implementation here just will not scale to the handling of all exceptions in all places in the code and will not therefore be maintainable. E.g. the PR suggests we handle ForbiddenException but what about ExpiredTokenException and NoCredentialsError and every possible other exception the client can throw if you haven't set up your AWS config properly?

I wonder if you should start by reviewing this much earlier PR
#997

[dboitnot]

In general I am a bit concerned that while I support the general idea, the implementation here just will not scale to the handling of all exceptions in all places in the code and will not therefore be maintainable.

I completely agree that this is addressing a narrow case and that a more general solution is needed. I would argue that:

1. The original implementation was broken, returning None when it should have raised the exception.
2. We needn't let perfect be the enemy of good. This PR fixes the bug and addresses an additional case which is affecting real users.
3. Misleading error messages are expensive and permissions issues are common.

C'mon, it's been forever since I submitted a PR. 😆

[alex-harvey-z3q]

@dboitnot Here are some more thoughts:

• Agree that the bug should be fixed and I think I agree about re-raising the exception.
• At a high level design point of view, I think we need to think carefully about whether we want to in general handle exceptions by sending hints back to the caller about how to setup their AWS and so on. Because if we handle this one lone exception this way, and then don't do the same everywhere else, that just gives the tool the feeling of an inconsistent design.
• If we do choose to do this, then I think the exception should be handled here https://github.com/Sceptre/sceptre/blob/master/sceptre/plan/actions.py#L309-L320
  or perhaps somewhere in here
  https://github.com/Sceptre/sceptre/blob/master/sceptre/connection_manager.py ?

[alex-harvey-z3q]

Closing in favour of #1530