lambci / docker-lambda

Docker images and test runners that replicate the live AWS Lambda environment
MIT License
5.82k stars 431 forks source link

Add null check to Log method #283

Closed AdamLewisGMSL closed 4 years ago

mhart commented 4 years ago

Awesome, thanks

mhart commented 4 years ago

So I'm taking a look at this – I think if I just initialize logs to be an empty string in the first place it should fix this – can you confirm on your end?

AdamLewisGMSL commented 4 years ago

Agree that would also work as a fix - I can confirm in about an hour if that's what you're wondering though?

mhart commented 4 years ago

Also, I think the timing issue could be fixed by adding:

context.StartTime = DateTime.Now;

To line 148, above:

context.RequestId = requestId;
context.DeadlineMs = long.Parse(deadlineMs);
context.Body = body;

Let me know if that fixes it – and if so, feel free to add to this PR (along with the string change) and I'll merge it in 👍

mhart commented 4 years ago

Made those changes and pushed up new images – let me know if they work for you

AdamLewisGMSL commented 4 years ago

I'll give this a test now, thanks for your help

Btw, I don't suppose you've been seeing issues with the 3.1 runtime & passing in a payload that contains spaces have you? Wrote and used my own serializer:

` namespace Temp { using System; using System.IO; using Amazon.Lambda.Core;

public class TempSerializer : ILambdaSerializer
{
    private Amazon.Lambda.Serialization.Json.JsonSerializer ser =
        new Amazon.Lambda.Serialization.Json.JsonSerializer();

    public new T Deserialize<T> (Stream requestStream)
    {
        requestStream.Seek(0, SeekOrigin.Begin);
        StreamReader reader = new StreamReader(requestStream);
        string text = reader.ReadToEnd();
        Console.WriteLine(text);
        requestStream.Seek(0, SeekOrigin.Begin);
        return this.ser.Deserialize<T>(requestStream);
    }

    public void Serialize<T>(T response, Stream responseStream)
    {
        ser.Serialize(response, responseStream);
    }
}

} `

And found serialization was failing because the Stream was only containing characters up to the first space... "{"key":"value"}" works, "{"key": "value"}" does not

By the way I'm only at the point of asking as we're currently trying to determine if it's our custom bash script entrypoint doing it or whether there's some difference between the two runtimes as they have different entrypoints by default now

mhart commented 4 years ago

How are you passing the event to docker-lambda? You're using stay-open mode yeah? So are you using the AWS CLI to do it, or curl, or what?

AdamLewisGMSL commented 4 years ago

For this particular use case we're passing straight through with a powershell script:

docker run --rm lamci_3_1 $invocationTarget ` $payload

Works with our locally tagged version of 2.1, for some reason throws errors with our version of 3.1. Suspect it's something to do with the entrypoint changing but need to confirm it yet, thought you might be able to advise?

ENTRYPOINT ["/var/lang/bin/dotnet", "/var/runtime/MockBootstraps.dll"] ENTRYPOINT ["/var/rapid/init", "--bootstrap", "/var/runtime/bootstrap", "--enable-msg-logs"]

mhart commented 4 years ago

If you're using stay-open mode, then you don't pass the payload when you run the container. You pass it each time you invoke

mhart commented 4 years ago

Also, just in case you're not running in stay-open mode and you do want to pass the event on the command line – are you sure you don't need to quote it in powershell? I don't know much about powershell, but in bash if you've got a variable that has spaces, then you definitely need to quote it. (ie, "$PAYLOAD")

mhart commented 4 years ago

You can also pipe the event into stdin using the DOCKER_LAMBDA_USE_STDIN env variable. See the bottom of the examples. https://github.com/lambci/docker-lambda#run-examples

AdamLewisGMSL commented 4 years ago

For this particular use case we're running in pass a payload & then exit mode, not the stay open + the powershell escaping is definitely working fine as the 2.1 runtime works perfectly

Will give the STDIN pipe a shot though to see if that helps simplify matters on our end so thanks for the suggestion :)

Btw, have tested the latest code, both the logs null & negative time issues are fixed :)