Skip to content

Conversation

vicheey
Copy link
Contributor

@vicheey vicheey commented May 12, 2025

Which issue(s) does this change fix?

#7927

Why is this change necessary?

From original issue:

When sending file (postman or frontend) to backend lambda through API Gateway I'm getting an error while doing local testing with sam local start-api

UnicodeDecodeError while processing HTTP request: 'utf-8' codec can't decode byte 0xb5 in position 473: invalid start byte.

How does it address the issue?

Update logic in event_constructor.py to decode request for various cases. .

What side effects does this change have?

  • This change is mostly additive and should not break existing functionality
  • It may change behavior for edge cases, but in ways that align better with API Gateway's actual behavior
  • The core logic for common cases remains the same

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Add input/output type hints to new functions/methods
  • Write design document if needed (Do I need to write a design document?)
  • Write/update unit tests
  • Write/update integration tests
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@vicheey
Copy link
Contributor Author

vicheey commented May 13, 2025

Want for #8019 to merge. This contains some fix for failing makr pr test.

@vicheey
Copy link
Contributor Author

vicheey commented May 19, 2025

Waiting for #8022 to merge before rerun the workflow.

@vicheey vicheey marked this pull request as ready for review May 20, 2025 15:36
@vicheey vicheey requested a review from a team as a code owner May 20, 2025 15:36
@valerena
Copy link
Contributor

I'm curious how much manual testing you did here, because the unit tests seem nice, but I don't really know if there are cases where the API request actually looks different than the mocked requests data that you added. Did you try this with curl/wget or another mechanism to actually send a binary file and confirmed that the file was received correctly? And tried with non-binary files, to confirm that we didn't break any of those?

Ideally you can run the local start-api integration tests in tests/integration/local/start_api/test_start_api.py to confirm that nothing broke.

But ideally we should have had a test before that should have failed because this behavior wasn't working. We do have some integ tests that check with a binary file, so it's weird that no test was catching this problem.

binary_data_file = "testdata/start_api/binarydata.gif"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants