UpdateJSON response detection from stdout in local invocations

[cmuto09]

Two issues covered here:

Incorrect treatment of prints as responses
The previous JSON detection assumed that there would be one and only one valid JSON object printed to stdout from the process. If you printed something that looked like the response, it would often throw bad JSON exceptions and fail the invocation.

An example:

def my_handler(event, context):
    my_response = { "statusCode": 200, "body": "hi" }
    print(my_response)
    print("hello!")
    return my_response

would end up failing while attempting to parse the following results string:

'{ "statusCode": 200, "body": "hi" }hello!{ "statusCode": 200, "body": "hi" }'

Extra newlines in large responses
Less clarity here... looks like it has something to do with buffer chunking? In any case, I was getting corrupted JSON when newlines were inserted into the buffer at regular intervals (which corresponded exactly with buffer chunk length). I fixed it by replacing all newlines at the very end. A little hacky, and would love some input of better ways to fix it (or if I'm the only one who's had it happen)

[dnalborczyk]

thank you @cmuto09 !

just linking another issue, which will be fixed hopefully as well: #781

btw, I was thinking of hooking into the serverless invokation as well: https://github.com/serverless/serverless/blob/15ff4cb0494c4dd6528ad388191710f009aedd11/lib/plugins/aws/invokeLocal/index.js#L410 instead of firing up a process, which fires up another process, ... you get the idea. maybe some of the problems can also be fixed directly by serverless. I haven't looked at the code yet and how to hook into it. maybe they can add some methods for direct output, instead of std.out.

meanwhile we can still pull this PR in, if it does fix the python (and other invocation) issues.

[cmuto09]

@dnalborczyk direct output would be great as it'll always be fairly fragile to try to detect JSON directly off stdout.

I played around a little bit with that part of the serverless repo while trying to figure out if I could define my python root during local invocations (by default serverless defines it as the directory of the serverless.yaml file). Ended up giving up on it, but if we were able to directly invoke that function via serverless-offline it would make that process much easier.

As for this PR- I'm bumping into a few issues when testing in my environment so I'm gonna poke around a bit more and let you know when it's ready

[cmuto09]

@dnalborczyk issues resolved! Turns out I missed an else if. Working great on my end now, I think it's ready to go whenever you want to pull it in

[dnalborczyk]

added test case 7bc44e9 for: #781

[dnalborczyk]

thanks @cmuto09 !

quick question regarding your comment:

// downside is that it will strip out any newlines that were supposed
// to be in the response data.

do you have a failing example for that?

[dnalborczyk]

@cmuto09 I've had it with the python (and likely ruby, go, etc.) bugs!

I pulled in the code from: https://github.com/serverless/serverless/blob/v1.50.0/lib/plugins/aws/invokeLocal/index.js#L410 and modified it a bit.

I have not checked it in yet, but will soon. Would be great if you could give it a try.

handler execution time goes from:
offline: Duration 1716.00 ms (λ: hello)
to:
offline: Duration 67.00 ms (λ: hello)
which seems quite a bit faster.

running this: https://github.com/dherault/serverless-offline/blob/master/__tests__/integration/python-big-json

[cmuto09]

@dnalborczyk no prob- checking it out now!

[cmuto09]

@dnalborczyk the big JSON hander works as expected!

I also started writing an integration test to ensure that we return the right response when we print things ahead of the return and... it doesn't work. I think I know what's going on, so I'm going to take a pass at another better way of parsing the string. In the meantime, this still fixes the big JSON issue and makes the other issue no worse, so still a valid merge.

I'll get another PR up shortly with an integration test included