Skip to content

lambda: restore ListStaticEntitlements tolerance and fix log-based failure classification #1073

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.

Sign up for GitHub

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

Draft
johnallers wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

lambda: restore ListStaticEntitlements tolerance and fix log-based failure classification #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
198231a
lambda: restore ListStaticEntitlements tolerance and fix log-based cl…
johnallers Aug 10, 2026
88294dc
lambda: classify an absent RPC from the payload's own error type
johnallers Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 130 additions & 12 deletions pkg/lambda/grpc/failure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,20 @@ const (
// FailureClassHandled means the function returned an error response through
// the runtime API rather than crashing.
FailureClassHandled = "handled"
// FailureClassUnsupportedRPC means the connector could not resolve the
// request message's type URL against its proto registry, which is how an
// SDK older than the RPC fails. The RPC is absent, not broken.
FailureClassUnsupportedRPC = "unsupported_rpc"
)

// oomErrorType is the Error Type the AWS platform stamps on the REPORT line
// when it kills a sandbox for exceeding its memory limit.
const oomErrorType = "Runtime.OutOfMemory"

// protoResolveErrorType is the errorType a connector runtime reports when it
// cannot resolve a message's type URL against its proto registry.
const protoResolveErrorType = "prefixError"

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion (low-medium confidence): "prefixError" is the unexported struct name of google.golang.org/protobuf/internal/errors.prefixError (vendor/google.golang.org/protobuf/internal/errors/errors.go:24), surfaced only because aws-lambda-go stamps the Go type name into errorType. A rename or refactor in a routine protobuf-go bump silently reverts this whole fix to the original bug, with no compile error and no test failure. Worth noting the vendored source in the comment so a dependency bump has a chance of flagging it, and consider whether the "unable to resolve" + "type.googleapis.com/" text match alone is sufficient without the exact-type gate.


// LambdaInvokeFailure is a structured description of a failed Lambda invoke.
//
// Every field is derived from data the invoke already returns: the base64 tail
Expand Down Expand Up @@ -105,6 +113,13 @@ func (e *LambdaInvokeFailure) Code() codes.Code {
return codes.DeadlineExceeded
case FailureClassOOM:
return codes.ResourceExhausted
case FailureClassUnsupportedRPC:
// The connector predates the RPC, so every invoke fails identically
// until it is rebuilt. Unimplemented lets a caller skip the step the
// same way it would for any connector that does not serve the method,
// and survives error-string sanitization, which a substring match on
// the log summary does not.
return codes.Unimplemented
default:
return codes.Unknown
}
Expand All @@ -129,6 +144,8 @@ func (e *LambdaInvokeFailure) Error() string {
}
case FailureClassTimeout:
_, _ = b.WriteString("function timed out")
case FailureClassUnsupportedRPC:
_, _ = b.WriteString("function does not support this RPC")
default:
_, _ = b.WriteString("function returned error")
}
Expand Down Expand Up @@ -373,6 +390,15 @@ func extractMeaningfulLogLines(raw string) string {
func classifyLambdaFailure(functionError string, statusCode int32, payload []byte, rawLog string) *LambdaInvokeFailure {
report, haveReport := parseLambdaReportLine(rawLog)
errPayload := parseLambdaErrorPayload(payload)

// Two different views of the same log, on purpose.
//
// LogSummary is the one that may reach a customer-visible field, so it
// keeps the truncation pre-filter that drops a partial leading line.
// Classification reads the unfiltered lines instead: dropping a line is a
// sanitization decision, and a sanitization decision must never be able to
// change which failure class an invoke lands in.
signalLogs := extractMeaningfulLogLines(rawLog)
filteredLogs := extractMeaningfulLogLines(dropTruncatedFirstLine(rawLog))

failure := &LambdaInvokeFailure{
Expand All @@ -392,7 +418,15 @@ func classifyLambdaFailure(functionError string, statusCode int32, payload []byt
if haveReport && report.ErrorType != "" {
failure.ErrorType = report.ErrorType
}
failure.FailureClass = lambdaFailureClass(functionError, payload, filteredLogs, report, failure.ErrorType)
failure.FailureClass = lambdaFailureClass(
functionError,
payload,
signalLogs,
report,
failure.ErrorType,
errPayload.ErrorType,
failure.ErrorMessage,
)

return failure
}
Expand All @@ -402,29 +436,59 @@ func classifyLambdaFailure(functionError string, statusCode int32, payload []byt
// Timeout is checked first: a sandbox killed on its execution timeout can also
// show peak memory at its ceiling, which would otherwise trip the OOM
// memory-comparison fallback.
func lambdaFailureClass(functionError string, payload []byte, filteredLogs string, report lambdaReport, errorType string) string {
//
// errorType is the resolved type, where a REPORT line's platform verdict wins
// over the payload's. payloadErrorType is the function's own, always. The two
// are separate because the OOM signals want the platform's verdict while an
// absent RPC is a statement only the function can make: a REPORT line carrying
// any Error Type would otherwise overwrite it and lose the capability gap.
func lambdaFailureClass(
functionError string,
payload []byte,
signalLogs string,
report lambdaReport,
errorType string,
payloadErrorType string,
errorMessage string,
) string {
// Existing signal, unchanged: the platform writes this into the error
// payload on a hard timeout kill.
if strings.Contains(string(payload), "Task timed out after") {
return FailureClassTimeout
}
// Existing signal, unchanged: the function's own context deadline was
// exhausted and the connector logged it.
if strings.Contains(filteredLogs, `\"error\":\"context deadline exceeded\"`) {
if strings.Contains(signalLogs, `\"error\":\"context deadline exceeded\"`) {
return FailureClassTimeout
}
// New signal: newer runtimes stamp the outcome on the REPORT line.
if strings.EqualFold(report.Status, "timeout") {
return FailureClassTimeout
}

// A connector whose SDK predates the RPC cannot resolve the request
// message's type URL. Checked before the OOM signals because it is an exact
// match on the runtime's own verdict, where the memory fallback below is
// inferred.
if isUnresolvedTypeURL(payloadErrorType, errorMessage, signalLogs) {
return FailureClassUnsupportedRPC
}
Comment on lines 460 to +475

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion (medium confidence): the in-function deadline signal is read from the tail log, which this file already documents as possibly containing a previous invoke's output on a warm sandbox (see parseLambdaReportLine), and switching it to signalLogs widens that window by no longer dropping the truncated leading line. It runs before isUnresolvedTypeURL, which reads the current invoke's own error payload. So an old connector whose warm tail happens to carry a stale \"error\":\"context deadline exceeded\" classifies as timeout/DeadlineExceeded, the Unimplemented mapping never happens, and the ListStaticEntitlements tolerance this PR restores does not apply. Payload-derived signals are stronger than tail-log-derived ones; consider running isUnresolvedTypeURL ahead of the log-based deadline check.


// Primary OOM signal, from the REPORT line or the error payload.
if errorType == oomErrorType {
return FailureClassOOM
}
// Fallback for runtimes that report the kill without an Error Type: a
// failed invoke whose peak memory reached its ceiling was an OOM.
if strings.EqualFold(report.Status, "error") &&
//
// Gated on the error type not being the function's own. A process the
// platform killed reports either nothing or a Runtime.* type such as
// Runtime.ExitError ("signal: killed"), so the ceiling is the best
// explanation available. When the function surfaced its own error value it
// has already explained the failure, and peak memory sitting at the ceiling
// is a coincidence - a Go runtime routinely runs at its ceiling without
// being killed.
if !isFunctionErrorType(errorType) && strings.EqualFold(report.Status, "error") &&

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: this commit split the resolved and payload error types precisely because "a REPORT line carrying any Error Type would otherwise overwrite" the function's own verdict — but the OOM gate still reads the resolved errorType, so the same masking applies to it. The new test fixture on line 337 establishes that payload errorType: prefixError + REPORT Error Type: Runtime.ExitError is a real shape; with Max Memory Used >= Memory Size that shape makes isFunctionErrorType(errorType) false and the invoke is classified OOM (ResourceExhausted, "function ran out of memory") even though the function surfaced its own error value — exactly what the gate was added to prevent. Consider gating on !isFunctionErrorType(payloadErrorType), or at minimum add that permutation to the table so the choice is pinned. (Minor, same doc block: "payloadErrorType is the function's own, always" isn't quite true — the existing OOM fixtures carry the platform's Runtime.ExitError in the payload.)

report.MemorySizeMB > 0 && report.MaxMemoryUsedMB >= report.MemorySizeMB {
return FailureClassOOM
}
Expand All @@ -435,6 +499,38 @@ func lambdaFailureClass(functionError string, payload []byte, filteredLogs strin
return FailureClassUnhandled
}

// platformErrorTypePrefix marks the error types the AWS runtime generates
// itself, such as Runtime.ExitError and Runtime.OutOfMemory. Anything without
// it came from the function's own error value.
const platformErrorTypePrefix = "Runtime."

// isFunctionErrorType reports whether the error type came from the function
// rather than the platform. An absent type is not a function error: a hard kill
// leaves nothing behind.
func isFunctionErrorType(errorType string) bool {
return errorType != "" && !strings.HasPrefix(errorType, platformErrorTypePrefix)
}

// isUnresolvedTypeURL reports whether the runtime failed to resolve a message's
// type URL against its proto registry.
//
// Both the error payload's message and the log lines are searched because which
// one carries the text depends on how the runtime surfaced the failure. The
// type URL itself is deliberately not matched: every RPC added after a given
// connector's SDK version fails this same way, so pinning the check to one
// method name would leave the next one unhandled.
func isUnresolvedTypeURL(errorType string, errorMessage string, signalLogs string) bool {
if errorType != protoResolveErrorType {
return false
}
for _, s := range []string{errorMessage, signalLogs} {
if strings.Contains(s, "unable to resolve") && strings.Contains(s, "type.googleapis.com/") {

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion (medium confidence): the two strings.Contains calls are evaluated independently against the whole joined multi-line string, so "unable to resolve" on one log line and "type.googleapis.com/" on a completely unrelated line satisfy the check together. Given that a false positive here maps to codes.Unimplemented, and Unimplemented is silently tolerated in two places in the syncer (getResourceFromConnector returns nil, nil at pkg/sync/syncer.go:1409, and syncStaticEntitlementsForResourceType skips the step), a false positive turns a real failure into silently missing data. Consider scoping the match to errorMessage only, or requiring both substrings on the same line.

return true
}
}
return false
}

// lambdaLogTailTruncationThresholdBytes is the point at which a tail log may
// have been cut mid-line. AWS returns the last 4 KB of the execution log with
// LogType: Tail and cuts the window at a byte offset, not a line boundary, so a
Expand Down Expand Up @@ -493,20 +589,42 @@ func looksLikeLogLineStart(line string) bool {
return true
}
}
// A text-format runtime line, which the platform prefixes with an
// RFC3339 timestamp such as "2006-01-02T15:04:05.000Z".
// A text-format line, identified by the timestamp its writer puts in front.
return looksLikeTimestampPrefix(line)
}

// looksLikeTimestampPrefix reports whether a line opens with an RFC3339-style
// date, i.e. "NNNN-NN-NNT".
// timestampLayouts are the line-leading timestamp shapes that mark a whole log
// record, written with 'N' standing for any digit.
//
// Both entries matter. The AWS platform writes RFC3339, but the connector
// runtimes writing into this log use Go's standard logger, whose default prefix
// is "2006/01/02 15:04:05" - slashes and a space, not dashes and a "T".
// Recognising only RFC3339 classifies every one of those whole lines as a
// truncated fragment.
var timestampLayouts = []string{
"NNNN-NN-NNT",
"NNNN/NN/NN NN:NN:NN",
}

// looksLikeTimestampPrefix reports whether a line opens with a recognised
// timestamp layout.
func looksLikeTimestampPrefix(line string) bool {
const stamp = "NNNN-NN-NNT"
if len(line) < len(stamp) {
for _, layout := range timestampLayouts {
if matchesDigitLayout(line, layout) {
return true
}
}
return false
}

// matchesDigitLayout reports whether line starts with layout, where 'N' matches
// any digit and every other byte must match exactly.
func matchesDigitLayout(line string, layout string) bool {
if len(line) < len(layout) {
return false
}
for i := range len(stamp) {
want := stamp[i]
for i := range len(layout) {
want := layout[i]
got := line[i]
switch want {
case 'N':
Expand Down
134 changes: 134 additions & 0 deletions pkg/lambda/grpc/failure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,75 @@ func TestClassifyLambdaFailure(t *testing.T) {
wantCode: codes.Unknown,
wantLogSummary: "lambda-run: unexpected failure",
},
{
// A connector whose SDK predates the RPC cannot resolve the request
// message's type URL. The method is absent, not broken, so it maps
// to Unimplemented and the caller can skip the step.
name: "unresolved request type from an old connector sdk",
functionError: "Unhandled",
statusCode: 200,
payload: `{"errorMessage":"proto: (line 1:88): unable to resolve ` +
`\"type.googleapis.com/c1.connector.v2.ExampleServiceListExamplesRequest\": ` +
`\"not found\"","errorType":"prefixError"}`,
rawLog: "2026/05/22 21:50:47 unable to resolve type URL\n",

wantClass: FailureClassUnsupportedRPC,
wantCode: codes.Unimplemented,
wantErrorType: "prefixError",
wantLogSummary: "2026/05/22 21:50:47 unable to resolve type URL",
wantErrorMessage: `proto: (line 1:88): unable to resolve ` +
`"type.googleapis.com/c1.connector.v2.ExampleServiceListExamplesRequest": "not found"`,
},
{
// The memory fallback infers an OOM from peak memory reaching the
// ceiling. A function that surfaced its own error value already
// explained the failure, and a Go runtime routinely sits at its
// ceiling without being killed, so the ceiling proves nothing here.
name: "function error type at memory ceiling is not an oom",
functionError: "Unhandled",
statusCode: 200,
payload: `{"errorType":"prefixError","errorMessage":"malformed request"}`,
rawLog: "REPORT RequestId: abc-123\tDuration: 5000.00 ms\tBilled Duration: 5000 ms\t" +
"Memory Size: 128 MB\tMax Memory Used: 128 MB\tStatus: error\n",

wantClass: FailureClassUnhandled,
wantCode: codes.Unknown,
wantRequestID: "abc-123",
wantErrorType: "prefixError",
wantMemorySize: 128,
wantMaxMemory: 128,
wantDurationMS: 5000,
wantUtilization: 100,
wantErrorMessage: "malformed request",
},
{
// An absent RPC is a statement only the function can make, so the
// classification reads the payload's error type rather than the
// resolved one. A REPORT line carrying its own Error Type must not
// overwrite that and turn the capability gap back into a crash --
// the ErrorType field still reports the platform's verdict, because
// that is what the field means.
name: "report error type does not mask an unresolved request type",
functionError: "Unhandled",
statusCode: 200,
payload: `{"errorMessage":"proto: (line 1:88): unable to resolve ` +
`\"type.googleapis.com/c1.connector.v2.ExampleServiceListExamplesRequest\": ` +
`\"not found\"","errorType":"prefixError"}`,
rawLog: "REPORT RequestId: def-456\tDuration: 12.00 ms\tBilled Duration: 12 ms\t" +
"Memory Size: 128 MB\tMax Memory Used: 64 MB\tStatus: error\t" +
"Error Type: Runtime.ExitError\n",

wantClass: FailureClassUnsupportedRPC,
wantCode: codes.Unimplemented,
wantRequestID: "def-456",
wantErrorType: "Runtime.ExitError",
wantMemorySize: 128,
wantMaxMemory: 64,
wantDurationMS: 12,
wantErrorMessage: `proto: (line 1:88): unable to resolve ` +
`"type.googleapis.com/c1.connector.v2.ExampleServiceListExamplesRequest": "not found"`,
wantUtilization: 50,
},
}

for _, c := range cases {
Expand Down Expand Up @@ -449,6 +518,66 @@ func TestDropTruncatedFirstLine(t *testing.T) {
raw := strings.Repeat("x", lambdaLogTailTruncationThresholdBytes+10)
require.Equal(t, raw, dropTruncatedFirstLine(raw))
})

// Connector runtimes log through Go's standard logger, whose default prefix
// is "2006/01/02 15:04:05". Recognising only RFC3339 treated every one of
// those whole lines as a truncated fragment and dropped it.
t.Run("keeps a Go stdlib timestamped leading line in a full window", func(t *testing.T) {
var b strings.Builder
b.WriteString("2026/05/22 21:50:47 lambda-run: failed to get connector\n")
for b.Len() < lambdaLogTailTruncationThresholdBytes {
b.WriteString(`{"level":"debug","msg":"listing resources","duration_ms":12}` + "\n")
}
require.Equal(t, b.String(), dropTruncatedFirstLine(b.String()))
})
}

// TestClassifyLambdaFailureTruncationDoesNotChangeClass pins that sanitizing the
// log summary cannot change which class an invoke lands in.
//
// The truncation pre-filter drops a leading line it cannot recognise as a whole
// record. Classifying from that filtered text made the in-function timeout
// signal vanish whenever it landed on the first line of a full tail window,
// downgrading a retryable DeadlineExceeded into a terminal Unknown.
func TestClassifyLambdaFailureTruncationDoesNotChangeClass(t *testing.T) {
const deadlineMarker = `\"error\":\"context deadline exceeded\"`

fullTail := func(firstLine string) string {
var b strings.Builder
b.WriteString(firstLine)
b.WriteString("\n")
for b.Len() < lambdaLogTailTruncationThresholdBytes {
b.WriteString("2026/05/22 21:50:48 still listing resources\n")
}
b.WriteString(reportHealthy)
return b.String()
}

// A whole Go-stdlib line that the pre-filter used to misjudge as a fragment.
t.Run("whole leading line carrying the signal", func(t *testing.T) {
raw := fullTail(`2026/05/22 21:50:47 {"level":"error",` + deadlineMarker + `}`)
require.GreaterOrEqual(t, len(raw), lambdaLogTailTruncationThresholdBytes)

failure := classifyLambdaFailure("Unhandled", 200, nil, raw)
require.Equal(t, FailureClassTimeout, failure.FailureClass)
require.Equal(t, codes.DeadlineExceeded, failure.Code(),
"a timeout must stay retryable for the sync framework")
})

// A genuine mid-record fragment that still carries the signal. The pre-filter
// is right to keep this out of the summary and wrong to let that decision
// reach classification, so this case is covered by reading the raw log.
t.Run("truncated leading line carrying the signal", func(t *testing.T) {
fragment := `msg":"page timed out",` + deadlineMarker + `}`
raw := fullTail(fragment)
require.GreaterOrEqual(t, len(raw), lambdaLogTailTruncationThresholdBytes)

failure := classifyLambdaFailure("Unhandled", 200, nil, raw)
require.Equal(t, FailureClassTimeout, failure.FailureClass)
require.Equal(t, codes.DeadlineExceeded, failure.Code())
require.NotContains(t, failure.LogSummary, fragment,
"the fragment must still be kept out of the sanitizable summary")
})
}

// TestClassifyLambdaFailureTruncatedTailDoesNotLeak is the regression test for
Expand Down Expand Up @@ -483,6 +612,11 @@ func TestLooksLikeTimestampPrefix(t *testing.T) {
expected bool
}{
{line: "2026-05-22T21:50:47.000Z\tabc\tINFO\thello", expected: true},
// Go's standard logger default prefix, which connector runtimes emit.
{line: "2026/05/22 21:50:47 lambda-run: failed to get connector", expected: true},
{line: "2026/05/22 21:50:47", expected: true},
{line: "2026/05/22 21:50", expected: false},
{line: "2026/05/22", expected: false},
{line: "2026-05-22 21:50:47", expected: false},
{line: "202X-05-22T21:50:47.000Z", expected: false},
{line: "2026-05-22", expected: false},
Expand Down
Loading
Loading