Skip to content

Logging UnicodeDecodeError details in LocalApigwService._request_handler#4016

Merged
hawflau merged 1 commit intoaws:developfrom
sncf-connect-tech:log-unicode-decode-error
Jun 29, 2022
Merged

Logging UnicodeDecodeError details in LocalApigwService._request_handler#4016
hawflau merged 1 commit intoaws:developfrom
sncf-connect-tech:log-unicode-decode-error

Conversation

@Lucas-C
Copy link
Contributor

@Lucas-C Lucas-C commented Jun 29, 2022

Which issue(s) does this change fix?

No issue was open for this

Why is this change necessary?

It improves "debuggability".
I've lost quite some time before understanding that the data I was sending in a POST payload could not be properly decoded
(as part of #4017)

How does it address the issue?

In case of a text decoding error, this PR ensures the error is logged.

What side effects does this change have?

None

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.

@github-actions github-actions bot added area/local/invoke sam local invoke command area/local/start-api sam local start-api command area/local/start-invoke pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Jun 29, 2022
@Lucas-C Lucas-C force-pushed the log-unicode-decode-error branch from f6b2b1b to 3bef459 Compare June 29, 2022 12:53
@Lucas-C
Copy link
Contributor Author

Lucas-C commented Jun 29, 2022

Note that there is already a unit test covering this:
test_request_handler_errors_when_unable_to_read_binary_data()

I did not alter it as part of this PR, as I only added some logging

I'd be happy to make any changes required, if need be

Copy link
Contributor

@hawflau hawflau left a comment

Choose a reason for hiding this comment

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

LGTM!

@hawflau hawflau requested review from a team, jfuss and mndeveci and removed request for a team and jfuss June 29, 2022 16:59
@hawflau hawflau merged commit 1a12e1d into aws:develop Jun 29, 2022
sidhujus pushed a commit to sidhujus/aws-sam-cli that referenced this pull request Jul 6, 2022
…ler (aws#4016)

Co-authored-by: Lucas Cimon <ext.lucas.cimon@sncf.fr>
moelasmar added a commit that referenced this pull request Jul 14, 2022
* chore: fix canary issues (#3978)

* Update sam logs setup to align with function update

* fix: don't use same layername in the template otherwise they will override themselves

* fix build tests

* downgrade python version for windows tests & add missing fields for adl sync tests

* re-format

* chore: Clarify debug log message in build_context command (#3968)

Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* fix: clean build folder before starting sync operation (#3982)

* fix: clean build folder before starting sync operation

* use function or layer build folder

Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>

* fix: Updated default option to Y for sync and added green colour to success message (#3986)

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* fix: Use cache build instead of incremental when building in container (#3988)

* use cache build instead of incremental when building in container

* use self reference

* bug: make sure all pullers in child combine puller also get stopped (#3990)

make sure all pullers in child combine puller also get stopped

* feat: Add --resource option input validation for sync --code (#3992)

* Add resource type validation for sync --code

* Refactor to use click.Choice and fix integration tests

* Add help text in option description

* Update samcli/commands/sync/command.py

Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>

* Remove debug flag for clean integ test output

Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>

* chore: replace InvokeContextException with granular ones (#3966)

* chore: replace InvokeContextException with granular ones

* but InvokeContextException back and extend other classes from it

Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* chore(deps): bump Newtonsoft.Json in dotnet example (#3996)

Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) from 12.0.1 to 13.0.1.
- [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases)
- [Commits](JamesNK/Newtonsoft.Json@12.0.1...13.0.1)

---
updated-dependencies:
- dependency-name: Newtonsoft.Json
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com>

* fix: add sync state and context to check previous run details (#3989)

* fix: add sync state and context to check previous run details

* add unit tests

* fix unit tests & formatting

* add integration tests

* remove redundant initialization

Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* chore: Update lambda builder version to 1.18.0 (#3999)

* Update lambda builder dependency version

* Update reproducible-linux hash

* feat: Adding --s3-bucket to sam sync (#3977)

* Added --s3-bucket option to sam sync

* Updated base command for integ tests

* Changed s3_bucket argument to be optional string

* Changed --s3-bucket to use existing util and refactor to ignore callbacks

* Removed callback argument for s3_bucket_option()

* Reverted function back and refactor argument name

* Moved logic out of arguments

* Added integ test to check if --s3-bucket option is being used

* Removed bucket check and replacede with resource check

Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* chore: fix java integration tests to use java8 runtime (#4002)

* chore: fix java integration tests to use java8 runtime

* update pom.xml definition as well

Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com>

* fix: add try/except for parse_time which is used by logs and traces commands (#4006)

* fix: issues with parallel build when using compiled languages (#4008)

* fix: run layer builds and function builds in separate async contextx

* add passthrough methods

* fix method call

* update unit tests

* add integration tests

* remove comment

* feat: updating app templates repo hash with (b057f88e4d4fe04acecbc22bab6b12cfbf695761) (#4007)

Co-authored-by: GitHub Action <action@github.com>

* chore: bump version to 1.53.0 (#4014)

* chore: skip TestSyncAdlWithWatchStartWithNoDependencies on windows (#4013)

Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* feat: remove accelerate beta experimental flag (#3973) (#3981)

* chore: remove accelerate beta experimental flag (#3973)

* remove beta experimental flag

* fix the if check for name

* use text variable directly

* fix unit tests

* fix unit test and remove accelerate flags

* remove accelerate beta feature flag

* skip accelerate experimental test

* fix build tests

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>

* fix integration tests

* fix unit and integration tests

* reformat

* docs: Update logs and traces docs (#4009)

* Update logs and traces docs

* Fix typos

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>

* Logging UnicodeDecodeError details in LocalApigwService._request_handler (#4016)

Co-authored-by: Lucas Cimon <ext.lucas.cimon@sncf.fr>

* Fix infinite deploy loop on small stack edge case (#4005)

* Fix typo in doc

* Replace last_event_time with execution_time

To not skip events in case of throttle

* Update tests for execute_changeset and wait_for_execute

* Document execution_time and set a default value

Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>

* chore: README.md Updated typo fixes done. (#4032)

* README.md Updated 

Updated README.md typo fixes.

* Update README.md

* feat: updating app templates repo hash with (dca20ade1b290262b6457006b52abadeacdddaf8) (#4030)

Co-authored-by: GitHub Action <action@github.com>

* Adding blogposts and workshop material to readme (#3997)

* blogs

* Readme update

* more

* some more

* roadmap

* adding pip

* added powertools

* adding more outbound links

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Update README.md

Co-authored-by: Chris Rehn <crehn@outlook.com>

* Adding sam validate in help context in the CLI.

* updating per Mehmet's feedback

* adding recent launches to readme

* updating readme

* minor updates

* updated - removed beta keyword for accelerate

Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>

* chore: Bump SamTranslator version to 1.47.0 (#4051)

* fix: Don't use deps dir for non-cached builds (#4052)

* Don't use deps dir for non-cached builds

* Remove unused import

* Fix test, rename variable to cached

* Make default cached false

* Add function definition

* Remove debug prints

Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Co-authored-by: Dan Vicarel <shundra8820@gmail.com>
Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: Lucas <12496191+lucashuy@users.noreply.github.com>
Co-authored-by: mingkun2020 <68391979+mingkun2020@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Lucas Cimon <925560+Lucas-C@users.noreply.github.com>
Co-authored-by: Lucas Cimon <ext.lucas.cimon@sncf.fr>
Co-authored-by: Mickael Bourgois <35171168+BourgoisMickael@users.noreply.github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Prasad Jivane <prasadjivane@gmail.com>
Co-authored-by: Praneeta Prakash <praneeta.prakash@gmail.com>
Co-authored-by: Chris Rehn <crehn@outlook.com>
Co-authored-by: aws-sam-cli-bot <46753707+aws-sam-cli-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/local/invoke sam local invoke command area/local/start-api sam local start-api command area/local/start-invoke pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants