Skip to content
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.

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

raw_body property inconsistency #503

Closed
johntmyers opened this issue Aug 30, 2017 · 2 comments
Closed

raw_body property inconsistency #503

johntmyers opened this issue Aug 30, 2017 · 2 comments
Labels

Comments

@johntmyers
Copy link
Contributor

When no body is sent, and app.current_request.raw_body is used, there is inconsistent behavior between local Chalice testing and actually how it runs in Lambda.

For local testing, if no body is sent self._body is getting set to a string: {}

However, when no body is provided in an actual call to API gateway, self._body is None and there is an unhandled AttributeError exception when this executes: self._raw_body = self._body.encode('utf-8')

This effectively makes it hard for an app developer to gracefully handle a missing body from a call and will always yield a 500 error back to the user.

I personally see no reason to catch if body is None within local.py this effectively gives a false impression of how a request will be handled by Lambda itself. Instead the None type should be handled within the raw_body property IMO.

Relevant code in app.py:

@property
    def raw_body(self):
        if self._raw_body is None:
            if self._is_base64_encoded:
                self._raw_body = self._base64decode(self._body)
            elif not isinstance(self._body, bytes):
                self._raw_body = self._body.encode('utf-8')
            else:
                self._raw_body = self._body
        return self._raw_body

Relevant code in local.py:

def create_lambda_event(self, method, path, headers, body=None):
        # type: (str, str, Dict[str, str], str) -> EventType
        view_route = self._route_matcher.match_route(path)
        event = {
            'requestContext': {
                'httpMethod': method,
                'resourcePath': view_route.route,
                'identity': {
                    'sourceIp': self.LOCAL_SOURCE_IP
                },
            },
            'headers': dict(headers),
            'queryStringParameters': view_route.query_params,
            'pathParameters': view_route.captured,
            'stageVariables': {},
        }
        # if body is None:
        #    event['body'] = '{}'
        if self._is_binary(headers):
            event['body'] = base64.b64encode(body).decode('ascii')
            event['isBase64Encoded'] = True
        else:
            event['body'] = body
        return event
@johntmyers
Copy link
Contributor Author

Note that I commented out the setting of body to "{}" here to validate

@jamesls
Copy link
Member

jamesls commented Aug 30, 2017

Confirmed bug:

@app.route('/')
def index():
    return {'hello': repr(app.current_request.raw_body)}
Traceback (most recent call last):
  File "/var/task/chalice/app.py", line 647, in _get_view_function_response
    response = view_function(**function_args)
  File "/var/task/app.py", line 9, in index
    return {'hello': repr(app.current_request.raw_body)}
  File "/var/task/chalice/app.py", line 284, in raw_body
    self._raw_body = self._body.encode('utf-8')
AttributeError: 'NoneType' object has no attribute 'encode'

I think setting this value to None/empty string makes sense for both chalice local and on actual Lambda.

Thanks for reporting.

@jamesls jamesls added the bug label Aug 30, 2017
jamesls added a commit to jamesls/chalice that referenced this issue Aug 30, 2017
This also includes an update the local.py so we get
consistent behavior when no HTTP body is provided.

Fixes aws#503.
jamesls added a commit to jamesls/chalice that referenced this issue Aug 31, 2017
This also includes an update the local.py so we get
consistent behavior when no HTTP body is provided.

Fixes aws#503.
jamesls added a commit to jamesls/chalice that referenced this issue Aug 31, 2017
This also includes an update the local.py so we get
consistent behavior when no HTTP body is provided.

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

No branches or pull requests

2 participants