Skip to content

fix(event_handlers): handle lack of headers when using auto-compression feature#1325

Merged
heitorlessa merged 2 commits intoaws-powertools:developfrom
tbuatois:fix/compress-response-api-gateway
Jul 25, 2022
Merged

fix(event_handlers): handle lack of headers when using auto-compression feature#1325
heitorlessa merged 2 commits intoaws-powertools:developfrom
tbuatois:fix/compress-response-api-gateway

Conversation

@tbuatois
Copy link
Contributor

@tbuatois tbuatois commented Jul 20, 2022 •
edited
Loading

Issue number: #1327

Summary

This PR add the ability use compress=True in decorator without throwing an error for a request containing null headers

Changes

  • Add check for NoneType / null header in get_header_value function and return default if so
  • Add corresponding unit test test_compress_no_accept_encoding_null_headers

User experience

  • Before
    Error for a request with empty/null headers to a method with compress=True:
    'NoneType' object has no attribute 'items'", "errorType": "AttributeError"

That was due to an error from api_gateway.py at line 212:

 if self.route.compress and "gzip" in (event.get_header_value("accept-encoding", "") or ""):
  • After

No more error, return default value as expected.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 20, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 20, 2022

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 20, 2022

No related issues found. Please ensure there is an open issue related to this change to avoid significant delays or closure.

@github-actions github-actions bot added do-not-merge need-issue PRs that are missing related issues bug Something isn't working labels Jul 20, 2022
@tbuatois tbuatois force-pushed the fix/compress-response-api-gateway branch from c1e8779 to 981959a Compare July 20, 2022 09:56
@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022 •
edited
Loading

Codecov Report

Merging #1325 (281722d) into develop (ab23491) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1325   +/-   ##
========================================
  Coverage    99.88%   99.88%           
========================================
  Files          119      119           
  Lines         5427     5429    +2     
  Branches       619      620    +1     
========================================
+ Hits          5421     5423    +2     
  Misses           2        2           
  Partials         4        4
Impacted Files Coverage Δ
...lambda_powertools/utilities/data_classes/common.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4bbcc24...281722d. Read the comment docs.

@heitorlessa heitorlessa removed do-not-merge need-issue PRs that are missing related issues labels Jul 20, 2022
heitorlessa
heitorlessa suggested changes Jul 20, 2022
View reviewed changes
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

tiny change to address another potential error

@heitorlessa heitorlessa changed the title fix(event_handler): NoneType error for compress feature without headers fix(event_handlers): NoneType error for compress feature without headers Jul 20, 2022
@tbuatois @heitorlessa
check order changes
281722d 
Changing the order to prevent case_sensitive=True logic to kick in and fail on AttributeError, since None wouldn't have .get

Co-authored-by: Heitor Lessa <heitor.lessa@hotmail.com>
@heitorlessa heitorlessa changed the title fix(event_handlers): NoneType error for compress feature without headers fix(event_handlers): handle lack of headers when using auto-compression feature Jul 25, 2022
@heitorlessa heitorlessa merged commit 7414df7 into aws-powertools:develop Jul 25, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 25, 2022

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@heitorlessa heitorlessa heitorlessa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working size/S Denotes a PR that changes 10-29 lines, ignoring generated files. tests utilities

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tbuatois @codecov-commenter @heitorlessa