fixed app imort failures not being logged

[circulon]

This PR fixes the issue where no log messages are added to CloudWatch during the pre-start (app) setup.
This is a major issue when integration layers with the Lambda and import errors are thrown.

The messages will now be visible in the CloudWath log and an InternalServerError is returned to the client.

[logandk]

Thanks for the PR and sorry about the delay in reviewing it. Why is the InternalServerError exception being returned rather than raised?

[logandk]

Also, tests aren't passing with this change. Are you able to fix the tests?

[circulon]

Also, tests aren't passing with this change. Are you able to fix the tests?

The tests are failing due to a github actions deprecation which has nothing to do with the code in the PR.
output from the tests:
"Error: This request has been automatically failed because it uses a deprecated version of actions/cache: v2. Please update your workflow to use v3/v4 of actions/cache to avoid interruptions. Learn more: https://github.blog/changelog/2024-12-05-notice-of-upcoming-releases-and-breaking-changes-for-github-actions/#actions-cache-v1-v2-and-actions-toolkit-cache-package-closing-down"

I will have a look and see if I can adjust the action

[circulon]

Thanks for the PR and sorry about the delay in reviewing it. Why is the InternalServerError exception being returned rather than raised?

no worries ;)

The reason for returning an InternalServerError is that at the raised exception is not a valid response that is expected from a Lamfba and as such is ignored.
This is a "pre-start" exception which is handled before the wsgi app is fully initialised.

The previous logic emitted the trace which is not a valid lambda response and also is not logged so its not visible in Cloudwatch either.
The new logic throws an InternalServerError for the client and logs an exception which will appear in CloudWatch logas BEFORE the START log line

to replicate is simple to replicate.

without this PR

1. deploy a valid app with a deliberate import error included
2. invoke the app from the command line 'sls wsgi invoke -f ...`
3. see a short traceback that only shows the wsgi_hander trace
4. see no errors in the cloudwatch logs

with this PR

1. deploy a valid app with a deliberate import error included
2. invoke the app from the command line 'sls wsgi invoke -f ...`
3. see an Eror 500
4. see a logging exception message in CloudWatch Logs that provides actial info about what caused the exception

I hope this helps clarify why this is important for debugging runtime isses.
Import issues with missing packages in a Layer is a prime example.

[logandk]

Thanks for clarifying. I think we can release that change as a minor version, since it is a change in behavior but not backwards-incompatible in the general case and will not affect any existing and functioning deployments.

I did fix the deprecated action on master, but this is not what I was referring to. You should be able to run the tests locally and confirm that they are not passing with the changes in this PR. I believe there's a test that asserts that logging occurs and that an exception is raised when importing the app fails.

[circulon]

Thanks for clarifying. I think we can release that change as a minor version, since it is a change in behavior but not backwards-incompatible in the general case and will not affect any existing and functioning deployments.

sounds good to me

I did fix the deprecated action on master, but this is not what I was referring to. You should be able to run the tests locally and confirm that they are not passing with the changes in this PR. I believe there's a test that asserts that logging occurs and that an exception is raised when importing the app fails.

Aah ok ....
I will pull in any changes from master to grab the actions fixes

I Thought I ran the tests and they passed ....
I will check them again

[circulon]

@logandk
Thought I had checked the tests but apparently not ...

All tests passing locally now ;)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.19%. Comparing base (b4f704c) to head (d8cad14).
Report is 7 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #273   +/-   ##
=======================================
  Coverage   98.19%   98.19%           
=======================================
  Files           5        5           
  Lines         609      611    +2     
  Branches       75       75           
=======================================
+ Hits          598      600    +2     
  Misses         11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[logandk]

Thanks @circulon! Everything looks great, awesome work. I'll merge now and prepare a release.