Skip to content

Remove stack traces from HTTP responses #12938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Remove stack traces from HTTP responses #12938

wants to merge 1 commit into from

Conversation

sannya-singal
Copy link
Contributor

@sannya-singal sannya-singal commented Jul 31, 2025 •
edited
Loading

Motivation

As a part of the initiative for improving Localstack error messages, this PR aims to remove stack traces from HTTP responses.

Current Behaviour:

sannyasingal@Sannyas-MacBook-Pro ~ % awslocal organizations describe-organization

An error occurred (InternalError) when calling the DescribeOrganization operation (reached max retries: 4): exception while calling organizations.DescribeOrganization: Traceback (most recent call last):
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/rolo/gateway/chain.py", line 166, in handle
    handler(self, self.context, response)
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/aws/handlers/service.py", line 112, in __call__
    handler(chain, context, response)
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/aws/handlers/service.py", line 82, in __call__
    skeleton_response = self.skeleton.invoke(context)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/aws/skeleton.py", line 154, in invoke
    return self.dispatch_request(serializer, context, instance)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/aws/skeleton.py", line 168, in dispatch_request
    result = handler(context, instance) or {}
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/aws/forwarder.py", line 133, in _call
    return handler(context, req)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/aws/skeleton.py", line 118, in __call__
    return self.fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/pro/core/services/organizations/provider.py", line 304, in describe_organization
    a = org[0]
        ~~~^^^
TypeError: 'NoneType' object is not subscriptable

Expected Behaviour:

sannyasingal@Sannyas-MacBook-Pro ~ % awslocal organizations describe-organization

An error occurred (InternalError) when calling the DescribeOrganization operation (reached max retries: 4): exception while calling organizations.DescribeOrganization: 'NoneType' object is not subscriptable

Changes

This PR excludes stack traces from HTTP responses when DEBUG=1 mode is enabled, for DEBUG=0 we already do not log stack trace in the response.

Testing

The changes have been tested locally by deliberately introducing an error by assigning a = org[0] in describe_organization handler and run awslocal organizations describe-organization command.
(It was discussed that for now manual local testing is enough and we do not need any specific test case to validate this.)

⚠️ MERGE ONLY AFTER THE 4.7.0 release.

@sannya-singal sannya-singal added this to the 4.8 milestone Jul 31, 2025
@sannya-singal sannya-singal self-assigned this Jul 31, 2025
@sannya-singal sannya-singal added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jul 31, 2025
@localstack-bot
Copy link
Contributor

Currently, only patch changes are allowed on main. Your PR labels (semver: minor) indicate that it cannot be merged into the main at this time.

1 similar comment
@localstack-bot
Copy link
Contributor

Currently, only patch changes are allowed on main. Your PR labels (semver: minor) indicate that it cannot be merged into the main at this time.

@github-actions GitHub Actions
Copy link

Test Results - Preflight, Unit

21 992 tests  ±0   20 258 ✅ ±0   6m 29s ⏱️ +5s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 08fc74c. ± Comparison against base commit 476e7f3.

@github-actions GitHub Actions
Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 14s ⏱️ +6s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 08fc74c. ± Comparison against base commit 476e7f3.

@github-actions GitHub Actions
Copy link

Test Results (amd64) - Integration, Bootstrap

    5 files  ±0      5 suites  ±0   2h 19m 0s ⏱️ + 1m 13s
4 848 tests ±0  4 367 ✅ ±0  481 💤 ±0  0 ❌ ±0 
4 854 runs  ±0  4 367 ✅ ±0  487 💤 ±0  0 ❌ ±0 

Results for commit 08fc74c. ± Comparison against base commit 476e7f3.

@coveralls
Copy link

Coverage Status

coverage: 83.175% (+31.8%) from 51.395%
when pulling 08fc74c on rm-stack-traces-http-resp
into 476e7f3 on main.

@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 45m 21s ⏱️ +57s
4 489 tests ±0  4 161 ✅ ±0  328 💤 ±0  0 ❌ ±0 
4 491 runs  ±0  4 161 ✅ ±0  330 💤 ±0  0 ❌ ±0 

Results for commit 08fc74c. ± Comparison against base commit 476e7f3.

@sannya-singal sannya-singal requested a review from k-a-il July 31, 2025 08:54
@sannya-singal sannya-singal marked this pull request as ready for review July 31, 2025 08:54
@sannya-singal sannya-singal requested a review from thrau as a code owner July 31, 2025 08:54
@k-a-il k-a-il requested a review from bentsku July 31, 2025 08:56
thrau
thrau reviewed Jul 31, 2025
View reviewed changes
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general question: if we remove this, how does the devx look like for localstack developers? the stacktrace is added when DEBUG=1 is set so the stack traces appear in test failures directly, which has helped (at least me) in the past to find issues very quickly.

@bentsku
Copy link
Contributor

bentsku commented Jul 31, 2025 •
edited
Loading

general question: if we remove this, how does the devx look like for localstack developers? the stacktrace is added when DEBUG=1 is set so the stack traces appear in test failures directly, which has helped (at least me) in the past to find issues very quickly.

That's a good point: it returns the error directly in the client call, which allows the test annotation to show the error directly and not have to go through the logs of the running LocalStack instance.

I think there's a fine line with the customer running with DEBUG=1 and what we show. But returning stack traces in error is IMHO a behavior change that is a bit more intrusing that just the debug level.
Maybe this should be a different flag, and not only rely on the logging level, but with another environment variable that would be set to true while running tests? Something that we could directly automatically enable via a fixture? That way, we'd keep the internal DevX while still prevent customers to see those stack traces.

@thrau
Copy link
Member

thrau commented Aug 1, 2025 •
edited
Loading

that's fair. so perhaps instead of removing the behavior (returning stack traces) all together, we could tie it to our tests being run. for example os.environ[constants.ENV_INTERNAL_TEST_RUN], will be set for integration tests and we could use that instead of config.DEBUG

@sannya-singal sannya-singal marked this pull request as draft August 1, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrau thrau thrau left review comments

@k-a-il k-a-il Awaiting requested review from k-a-il

@bentsku bentsku Awaiting requested review from bentsku

At least 1 approving review is required to merge this pull request.

Assignees

@sannya-singal sannya-singal

Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
4.8
Development

Successfully merging this pull request may close these issues.
5 participants
@sannya-singal @localstack-bot @coveralls @bentsku @thrau