Repository navigation
Adding exception message to return to CFN - #171
JohnPreston wants to merge 2 commits into
Conversation
0734ea3 to
a149b5b
Compare
|
Rebased from this master branch after hooks changes. |
|
I believe that the current behavior is intentional as exception message may inadvertently leak data or implementation details (for closed source resources). I suggest doing catch-all error handling in your handler code that returns a failed |
|
Right. I see your point. However, without the message attribute one would not know what is happenning. I might be a weird edge case too but I am getting no log at all with 3 resources (in the log group) although they all past type testing. When used in CFN it totally fails and that is then the only way to get any indication of what might be wrong. Maybe worth rewriting this with a feature switch so that one does not have to rework the whole Edit: sorry misclicked on the close button :/ |
CW client is initialized on the runtime before handler method is invoked. If you dont get any logs it might indicate that error happened before request even reached your code. Such issues require debugging, so I would recommend creating a support ticket so CFN can securely investigate your private stacks and either provide a fix or a workaround. Let's keep this issue opened for visibility and progress update. |
|
@ammokhov thanks. If that helps |
|
PS: This PR / change I had to do to get any sort of feedback from the execution. So in CFN I do get the reason back with that change, but no matter where you set print/logging, nothing ever shows in CW Logs. |
a149b5b to
5b97aab
Compare
Yes, we've been contacted by Support regarding this issue (cc: #181, #182). We'll try to get some eyes on this as soon as we can. Side note: it looks like the build CI started breaking, so we'll have to fix that as well. (EDIT: #183) |
darrylabbate
left a comment
There was a problem hiding this comment.
I just want to echo @jaymccon's sentiment. These changes would significantly increase the risk of leaking implementation details for customers' resources, which is not something we want to impose on everyone who uses this library.
Even behind a feature flag/toggle (as you suggested), I feel this would be bad form overall.
|
totally fine, since then figured the #182 was the reason for the lack of return at all. |
Adding message to the progress event for
ExceptionandBaseException(strongly casted to str) to provide more insights on what the error was on__call__Tested in private resource in CFN (by bunding code manually vs using
cfn submit). Local sam tests andaws cloudformation test-typeboth pass with the change.