Skip to content

Fix oversized response handling#33

Merged
dominikschubert merged 4 commits into
localstackfrom
fix-oversized-response-handling
Feb 13, 2024
Merged

Fix oversized response handling#33
dominikschubert merged 4 commits into
localstackfrom
fix-oversized-response-handling

Conversation

@dominikschubert

Copy link
Copy Markdown
Member

When sending an oversized (>6MB) response payload, the runtime process exits with exit code 1. In our error handling code we used log.Fatal when handling a ErrInvokeDoneFailed exception which caused the init to prematurely exit and thus didn't send a proper response back to localstack.

Additionaly when testing, I've noticed a small parity gap, so I patched the generated message to not include the actual bytes there. Though I guess its debatable if we want to keep that part of the PR or not. ���‍♂️

@dfangl dfangl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good in principal, just minor nits

Comment thread cmd/localstack/custom_interop.go
Comment thread cmd/localstack/main.go
Comment thread debugging/Makefile Outdated
Comment thread lambda/core/directinvoke/directinvoke.go
@dominikschubert dominikschubert merged commit ddc62d9 into localstack Feb 13, 2024
@dominikschubert dominikschubert deleted the fix-oversized-response-handling branch February 13, 2024 16:04

@joe4dev joe4dev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🚀

Comment thread cmd/localstack/main.go
Comment thread cmd/localstack/main.go
User: GetenvWithDefault("LOCALSTACK_USER", "sbx_user1051"),
InitLogLevel: GetenvWithDefault("LOCALSTACK_INIT_LOG_LEVEL", "warn"),
EdgePort: GetenvWithDefault("EDGE_PORT", "4566"),
MaxPayloadSize: GetenvWithDefault("LOCALSTACK_MAX_PAYLOAD_SIZE", "6291556"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Intentionally no unit _BYTES vs. LAMBDA_LIMITS_MAX_FUNCTION_PAYLOAD_SIZE_BYTES in LocalStack
guess to match with interop.MaxPayloadSize

(LOCALSTACK_POST_INVOKE_WAIT_MS also has a unit)

Comment thread lambda/interop/model.go
log "github.com/sirupsen/logrus"
)

var MaxPayloadSize int = 6*1024*1024 + 100 // 6 MiB + 100 bytes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

annoying that we cannot use a single type due to mixed usages 😢 (server.go requires an int; not int64)

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

Labels

None yet

3 participants