Skip to content

Commit 36eae1a

Browse files
author
Alex Wang
committed
fix(plugin): guard result on success, keep it out of toString, keep the old arity
Review feedback on the result accessor: - Only a SUCCEEDED operation reports a result. A wait-for-condition is checkpointed as STEP and reuses stepDetails().result() to carry its intermediate check-loop state between attempts, so a failed one could surface that state as its result, contradicting the documented null-on-failure contract. Verified: without the guard the new test reports {"polls":2} for a FAILED operation. - Override toString() to omit result. Records render every component, so plugins that log the info object whole would have begun emitting customer payloads (possibly secrets or personal data). Output is otherwise unchanged; matches the invocation records from #622. - Add a constructor at the previous 11-argument arity delegating with a null result, so existing callers keep compiling and linking. - Cover the CHAINED_INVOKE, CALLBACK, and CONTEXT extraction branches, which had no direct test.
1 parent 1232c18 commit 36eae1a

3 files changed

Lines changed: 156 additions & 2 deletions

File tree

‎sdk/src/main/java/software/amazon/lambda/durable/plugin/OperationEndInfo.java‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,52 @@ public record OperationEndInfo(
3434
Integer attempt,
3535
boolean isReplay,
3636
@Experimental Throwable error,
37-
@Experimental String result) {}
37+
@Experimental String result) {
38+
39+
/**
40+
* Creates operation-end information without a result.
41+
*
42+
* <p>Retained so callers written before {@code result} was added keep compiling and linking; {@code result}
43+
* resolves to null.
44+
*
45+
* @param id operation ID
46+
* @param name human-readable operation name (may be null)
47+
* @param type operation type
48+
* @param subType operation sub-type (may be null)
49+
* @param parentId parent operation ID (null for root-level operations)
50+
* @param startTimestamp when the operation started
51+
* @param endTimestamp when the operation ended
52+
* @param status the operation's terminal status
53+
* @param attempt the total number of attempts for retriable operations
54+
* @param isReplay true if this operation already existed in the execution state
55+
* @param error non-null if the operation failed
56+
*/
57+
public OperationEndInfo(
58+
String id,
59+
String name,
60+
String type,
61+
String subType,
62+
String parentId,
63+
Instant startTimestamp,
64+
Instant endTimestamp,
65+
String status,
66+
Integer attempt,
67+
boolean isReplay,
68+
Throwable error) {
69+
this(id, name, type, subType, parentId, startTimestamp, endTimestamp, status, attempt, isReplay, error, null);
70+
}
71+
72+
/**
73+
* Returns a representation that omits {@code result}.
74+
*
75+
* <p>The generated representation would render the operation result, so plugins that log this object whole would
76+
* start emitting customer payloads, potentially including secrets or personal data. Read the component explicitly
77+
* to record it.
78+
*/
79+
@Override
80+
public String toString() {
81+
return "OperationEndInfo[id=" + id + ", name=" + name + ", type=" + type + ", subType=" + subType
82+
+ ", parentId=" + parentId + ", startTimestamp=" + startTimestamp + ", endTimestamp=" + endTimestamp
83+
+ ", status=" + status + ", attempt=" + attempt + ", isReplay=" + isReplay + ", error=" + error + "]";
84+
}
85+
}

‎sdk/src/main/java/software/amazon/lambda/durable/plugin/PluginInfoConverter.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.util.Collection;
77
import java.util.stream.Collectors;
88
import software.amazon.awssdk.services.lambda.model.Operation;
9+
import software.amazon.awssdk.services.lambda.model.OperationStatus;
910
import software.amazon.lambda.durable.model.OperationIdentifier;
1011
import software.amazon.lambda.durable.operation.BaseDurableOperation;
1112

@@ -71,9 +72,14 @@ public static OperationEndInfo toOperationEndInfo(
7172
/**
7273
* Extracts the serialized result from an operation based on its type. Returns null if the operation has no result
7374
* (e.g., failed or still running).
75+
*
76+
* <p>Only a SUCCEEDED operation reports a result. The status guard is required, not just defensive: a
77+
* wait-for-condition is checkpointed as {@link OperationType#STEP} and reuses {@code stepDetails().result()} to
78+
* carry its intermediate check-loop state between attempts, so a failed one can still hold state that must not be
79+
* surfaced as that operation's result.
7480
*/
7581
private static String extractResult(Operation operation) {
76-
if (operation == null || operation.type() == null) {
82+
if (operation == null || operation.type() == null || operation.status() != OperationStatus.SUCCEEDED) {
7783
return null;
7884
}
7985
return switch (operation.type()) {

‎sdk/src/test/java/software/amazon/lambda/durable/plugin/PluginInfoConverterTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
import java.time.Instant;
88
import org.junit.jupiter.api.Test;
9+
import software.amazon.awssdk.services.lambda.model.CallbackDetails;
10+
import software.amazon.awssdk.services.lambda.model.ChainedInvokeDetails;
11+
import software.amazon.awssdk.services.lambda.model.ContextDetails;
912
import software.amazon.awssdk.services.lambda.model.Operation;
1013
import software.amazon.awssdk.services.lambda.model.OperationStatus;
1114
import software.amazon.awssdk.services.lambda.model.OperationType;
@@ -113,6 +116,103 @@ void toOperationEndInfo_extractsResult_fromSucceededStep() {
113116
assertEquals("\"hello\"", info.result());
114117
}
115118

119+
@Test
120+
void toOperationEndInfo_extractsResult_fromSucceededChainedInvoke() {
121+
var operation = Operation.builder()
122+
.startTimestamp(START)
123+
.endTimestamp(END)
124+
.type(OperationType.CHAINED_INVOKE)
125+
.status(OperationStatus.SUCCEEDED)
126+
.chainedInvokeDetails(
127+
ChainedInvokeDetails.builder().result("\"invoked\"").build())
128+
.build();
129+
130+
var info = PluginInfoConverter.toOperationEndInfo(operation, STEP_IDENTIFIER, null, false, null);
131+
132+
assertEquals("\"invoked\"", info.result());
133+
}
134+
135+
@Test
136+
void toOperationEndInfo_extractsResult_fromSucceededCallback() {
137+
var operation = Operation.builder()
138+
.startTimestamp(START)
139+
.endTimestamp(END)
140+
.type(OperationType.CALLBACK)
141+
.status(OperationStatus.SUCCEEDED)
142+
.callbackDetails(
143+
CallbackDetails.builder().result("\"called-back\"").build())
144+
.build();
145+
146+
var info = PluginInfoConverter.toOperationEndInfo(operation, STEP_IDENTIFIER, null, false, null);
147+
148+
assertEquals("\"called-back\"", info.result());
149+
}
150+
151+
@Test
152+
void toOperationEndInfo_extractsResult_fromSucceededContext() {
153+
var operation = Operation.builder()
154+
.startTimestamp(START)
155+
.endTimestamp(END)
156+
.type(OperationType.CONTEXT)
157+
.status(OperationStatus.SUCCEEDED)
158+
.contextDetails(
159+
ContextDetails.builder().result("\"child-done\"").build())
160+
.build();
161+
162+
var info = PluginInfoConverter.toOperationEndInfo(operation, STEP_IDENTIFIER, null, false, null);
163+
164+
assertEquals("\"child-done\"", info.result());
165+
}
166+
167+
@Test
168+
void toOperationEndInfo_resultIsNull_whenFailedWaitForConditionRetainsCheckpointState() {
169+
// A wait-for-condition is checkpointed as STEP and reuses stepDetails().result() to carry its
170+
// intermediate check-loop state between attempts, so a failed one can still hold state.
171+
var operation = Operation.builder()
172+
.startTimestamp(START)
173+
.endTimestamp(END)
174+
.type(OperationType.STEP)
175+
.status(OperationStatus.FAILED)
176+
.stepDetails(
177+
StepDetails.builder().attempt(3).result("{\"polls\":2}").build())
178+
.build();
179+
180+
var info = PluginInfoConverter.toOperationEndInfo(operation, STEP_IDENTIFIER, null, false, null);
181+
182+
assertNull(info.result(), "a failed operation must not report intermediate state as its result");
183+
}
184+
185+
@Test
186+
void operationEndInfo_compatibilityConstructor_leavesResultNull() {
187+
var info = new OperationEndInfo(
188+
OPERATION_ID, OPERATION_NAME, "STEP", "Step", PARENT_ID, START, END, "SUCCEEDED", 1, false, null);
189+
190+
assertNull(info.result());
191+
}
192+
193+
@Test
194+
void operationEndInfo_toString_omitsResult() {
195+
var info = new OperationEndInfo(
196+
OPERATION_ID,
197+
OPERATION_NAME,
198+
"STEP",
199+
"Step",
200+
PARENT_ID,
201+
START,
202+
END,
203+
"SUCCEEDED",
204+
1,
205+
false,
206+
null,
207+
"s3cret-result");
208+
209+
var rendered = info.toString();
210+
211+
assertFalse(rendered.contains("s3cret-result"), "operation result must not leak into logs");
212+
assertTrue(rendered.contains(OPERATION_ID));
213+
assertTrue(rendered.contains("SUCCEEDED"));
214+
}
215+
116216
@Test
117217
void toOperationEndInfo_resultIsNull_whenOperationHasNoResult() {
118218
var operation = Operation.builder()

0 commit comments

Comments
 (0)