Skip to content

Commit df2e5ea

Browse files
jckingcopybara-github
authored andcommitted
Fix regression related to errors when selecting missing map entries or struct fields
PiperOrigin-RevId: 688363906
1 parent c3a0e9f commit df2e5ea

2 files changed

Lines changed: 22 additions & 12 deletions

File tree

eval/eval/select_step.cc

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,16 +223,22 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const {
223223
switch (arg->kind()) {
224224
case ValueKind::kStruct: {
225225
Value result;
226-
CEL_RETURN_IF_ERROR(arg.GetStruct().GetFieldByName(
227-
frame->value_factory(), field_, result, unboxing_option_));
226+
auto status = arg.GetStruct().GetFieldByName(
227+
frame->value_factory(), field_, result, unboxing_option_);
228+
if (!status.ok()) {
229+
result = ErrorValue(std::move(status));
230+
}
228231
frame->value_stack().PopAndPush(std::move(result),
229232
std::move(result_trail));
230233
return absl::OkStatus();
231234
}
232235
case ValueKind::kMap: {
233236
Value result;
234-
CEL_RETURN_IF_ERROR(
235-
arg.GetMap().Get(frame->value_factory(), field_value_, result));
237+
auto status =
238+
arg.GetMap().Get(frame->value_factory(), field_value_, result);
239+
if (!status.ok()) {
240+
result = ErrorValue(std::move(status));
241+
}
236242
frame->value_stack().PopAndPush(std::move(result),
237243
std::move(result_trail));
238244
return absl::OkStatus();
@@ -372,7 +378,11 @@ class DirectSelectStep : public DirectExpressionStep {
372378
return PerformOptionalSelect(frame, optional_arg->Value(), result);
373379
}
374380

375-
return PerformSelect(frame, result, result);
381+
auto status = PerformSelect(frame, result, result);
382+
if (!status.ok()) {
383+
result = ErrorValue(std::move(status));
384+
}
385+
return absl::OkStatus();
376386
}
377387

378388
private:

eval/eval/select_step_test.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -738,16 +738,16 @@ TEST_P(SelectStepConformanceTest, CustomAccessorErrorHandling) {
738738

739739
// For get field, implementation may return an error-type cel value or a
740740
// status (e.g. broken assumption using a core type).
741-
ASSERT_THAT(RunExpression(value, "message_value",
742-
/*test=*/false,
743-
/*unknown_path=*/"", options),
744-
StatusIs(absl::StatusCode::kInternal));
745-
746-
// testonly select (has) errors are coerced to CelError.
747741
ASSERT_OK_AND_ASSIGN(CelValue result,
748742
RunExpression(value, "message_value",
749-
/*test=*/true,
743+
/*test=*/false,
750744
/*unknown_path=*/"", options));
745+
EXPECT_THAT(result, test::IsCelError(StatusIs(absl::StatusCode::kInternal)));
746+
747+
// testonly select (has) errors are coerced to CelError.
748+
ASSERT_OK_AND_ASSIGN(result, RunExpression(value, "message_value",
749+
/*test=*/true,
750+
/*unknown_path=*/"", options));
751751

752752
EXPECT_THAT(result, test::IsCelError(StatusIs(absl::StatusCode::kNotFound)));
753753
}

0 commit comments

Comments
 (0)