Skip to content

Commit 2189f32

Browse files
l46kokcopybara-github
authored andcommitted
Improve the error message in planned CreateMap when duplicate key exists
PiperOrigin-RevId: 851474395
1 parent efba831 commit 2189f32

File tree

11 files changed

+120
-98
lines changed

11 files changed

+120
-98
lines changed

common/exceptions/BUILD.bazel

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ java_library(
5353
exports = ["//common/src/main/java/dev/cel/common/exceptions:iteration_budget_exceeded"],
5454
)
5555

56+
java_library(
57+
name = "duplicate_key",
58+
# used_by_android
59+
exports = ["//common/src/main/java/dev/cel/common/exceptions:duplicate_key"],
60+
)
61+
5662
java_library(
5763
name = "overload_not_found",
5864
# used_by_android

common/src/main/java/dev/cel/common/exceptions/BUILD.bazel

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,19 @@ java_library(
111111
],
112112
)
113113

114+
java_library(
115+
name = "duplicate_key",
116+
srcs = ["CelDuplicateKeyException.java"],
117+
# used_by_android
118+
tags = [
119+
],
120+
deps = [
121+
":runtime_exception",
122+
"//common:error_codes",
123+
"//common/annotations",
124+
],
125+
)
126+
114127
java_library(
115128
name = "overload_not_found",
116129
srcs = ["CelOverloadNotFoundException.java"],
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package dev.cel.common.exceptions;
16+
17+
import dev.cel.common.CelErrorCode;
18+
import dev.cel.common.annotations.Internal;
19+
20+
/** Indicates an attempt to create a map using duplicate keys. */
21+
@Internal
22+
public final class CelDuplicateKeyException extends CelRuntimeException {
23+
24+
public static CelDuplicateKeyException of(Object key) {
25+
return new CelDuplicateKeyException(String.format("duplicate map key [%s]", key));
26+
}
27+
28+
private CelDuplicateKeyException(String message) {
29+
super(message, CelErrorCode.DUPLICATE_ATTRIBUTE);
30+
}
31+
}

runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ java_library(
6363
deps = [
6464
":error_metadata",
6565
":execution_frame",
66+
":localized_evaluation_exception",
6667
":planned_interpretable",
67-
":strict_error_exception",
6868
"//:auto_value",
6969
"//common:options",
7070
"//common/exceptions:runtime_exception",
@@ -87,8 +87,6 @@ java_library(
8787
deps = [
8888
":execution_frame",
8989
":planned_interpretable",
90-
"//common/values",
91-
"//common/values:cel_byte_string",
9290
"//runtime:interpretable",
9391
"@maven//:com_google_errorprone_error_prone_annotations",
9492
"@maven//:com_google_guava_guava",
@@ -321,7 +319,9 @@ java_library(
321319
srcs = ["EvalCreateMap.java"],
322320
deps = [
323321
":execution_frame",
322+
":localized_evaluation_exception",
324323
":planned_interpretable",
324+
"//common/exceptions:duplicate_key",
325325
"//runtime:evaluation_exception",
326326
"//runtime:interpretable",
327327
"@maven//:com_google_errorprone_error_prone_annotations",
@@ -361,8 +361,8 @@ java_library(
361361
srcs = ["EvalHelpers.java"],
362362
deps = [
363363
":execution_frame",
364+
":localized_evaluation_exception",
364365
":planned_interpretable",
365-
":strict_error_exception",
366366
"//common:error_codes",
367367
"//common/exceptions:runtime_exception",
368368
"//common/values",
@@ -371,8 +371,8 @@ java_library(
371371
)
372372

373373
java_library(
374-
name = "strict_error_exception",
375-
srcs = ["StrictErrorException.java"],
374+
name = "localized_evaluation_exception",
375+
srcs = ["LocalizedEvaluationException.java"],
376376
deps = [
377377
"//common:error_codes",
378378
"//common/exceptions:runtime_exception",

runtime/src/main/java/dev/cel/runtime/planner/EvalConstant.java

Lines changed: 4 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,12 @@
1414

1515
package dev.cel.runtime.planner;
1616

17-
import com.google.common.primitives.UnsignedLong;
1817
import com.google.errorprone.annotations.Immutable;
19-
import dev.cel.common.values.CelByteString;
20-
import dev.cel.common.values.NullValue;
2118
import dev.cel.runtime.GlobalResolver;
2219

2320
@Immutable
2421
final class EvalConstant extends PlannedInterpretable {
2522

26-
// Pre-allocation of common constants
27-
private static final EvalConstant NULL_VALUE = new EvalConstant(NullValue.NULL_VALUE);
28-
private static final EvalConstant TRUE = new EvalConstant(true);
29-
private static final EvalConstant FALSE = new EvalConstant(false);
30-
private static final EvalConstant ZERO = new EvalConstant(0L);
31-
private static final EvalConstant ONE = new EvalConstant(1L);
32-
private static final EvalConstant UNSIGNED_ZERO = new EvalConstant(UnsignedLong.ZERO);
33-
private static final EvalConstant UNSIGNED_ONE = new EvalConstant(UnsignedLong.ONE);
34-
private static final EvalConstant EMPTY_STRING = new EvalConstant("");
35-
private static final EvalConstant EMPTY_BYTES = new EvalConstant(CelByteString.EMPTY);
36-
3723
@SuppressWarnings("Immutable") // Known CEL constants that aren't mutated are stored
3824
private final Object constant;
3925

@@ -42,59 +28,12 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) {
4228
return constant;
4329
}
4430

45-
static EvalConstant create(boolean value) {
46-
return value ? TRUE : FALSE;
47-
}
48-
49-
static EvalConstant create(String value) {
50-
if (value.isEmpty()) {
51-
return EMPTY_STRING;
52-
}
53-
54-
return new EvalConstant(value);
55-
}
56-
57-
static EvalConstant create(long value) {
58-
if (value == 0L) {
59-
return ZERO;
60-
} else if (value == 1L) {
61-
return ONE;
62-
}
63-
64-
return new EvalConstant(Long.valueOf(value));
65-
}
66-
67-
static EvalConstant create(double value) {
68-
return new EvalConstant(Double.valueOf(value));
69-
}
70-
71-
static EvalConstant create(UnsignedLong unsignedLong) {
72-
if (unsignedLong.longValue() == 0L) {
73-
return UNSIGNED_ZERO;
74-
} else if (unsignedLong.longValue() == 1L) {
75-
return UNSIGNED_ONE;
76-
}
77-
78-
return new EvalConstant(unsignedLong);
79-
}
80-
81-
static EvalConstant create(NullValue unused) {
82-
return NULL_VALUE;
83-
}
84-
85-
static EvalConstant create(CelByteString byteString) {
86-
if (byteString.isEmpty()) {
87-
return EMPTY_BYTES;
88-
}
89-
return new EvalConstant(byteString);
90-
}
91-
92-
static EvalConstant create(Object value) {
93-
return new EvalConstant(value);
31+
static EvalConstant create(long exprId, Object value) {
32+
return new EvalConstant(exprId, value);
9433
}
9534

96-
private EvalConstant(Object constant) {
97-
super(/* exprId= */ -1); // It's not possible to throw while evaluating a constant
35+
private EvalConstant(long exprId, Object constant) {
36+
super(exprId);
9837
this.constant = constant;
9938
}
10039
}

runtime/src/main/java/dev/cel/runtime/planner/EvalCreateMap.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
import com.google.common.base.Preconditions;
1818
import com.google.common.collect.ImmutableMap;
19+
import com.google.common.collect.Sets;
1920
import com.google.errorprone.annotations.Immutable;
21+
import dev.cel.common.exceptions.CelDuplicateKeyException;
2022
import dev.cel.runtime.CelEvaluationException;
2123
import dev.cel.runtime.GlobalResolver;
24+
import java.util.HashSet;
2225

2326
@Immutable
2427
final class EvalCreateMap extends PlannedInterpretable {
@@ -35,13 +38,23 @@ final class EvalCreateMap extends PlannedInterpretable {
3538
public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEvaluationException {
3639
ImmutableMap.Builder<Object, Object> builder =
3740
ImmutableMap.builderWithExpectedSize(keys.length);
41+
HashSet<Object> keysSeen = Sets.newHashSetWithExpectedSize(keys.length);
42+
3843
for (int i = 0; i < keys.length; i++) {
39-
builder.put(keys[i].eval(resolver, frame), values[i].eval(resolver, frame));
44+
Object key = keys[i].eval(resolver, frame);
45+
46+
if (!keysSeen.add(key)) {
47+
throw new LocalizedEvaluationException(CelDuplicateKeyException.of(key), keys[i].exprId());
48+
}
49+
50+
builder.put(key, values[i].eval(resolver, frame));
4051
}
52+
4153
return builder.buildOrThrow();
4254
}
4355

44-
static EvalCreateMap create(long exprId, PlannedInterpretable[] keys, PlannedInterpretable[] values) {
56+
static EvalCreateMap create(
57+
long exprId, PlannedInterpretable[] keys, PlannedInterpretable[] values) {
4558
return new EvalCreateMap(exprId, keys, values);
4659
}
4760

runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ static Object evalNonstrictly(
2525
PlannedInterpretable interpretable, GlobalResolver resolver, ExecutionFrame frame) {
2626
try {
2727
return interpretable.eval(resolver, frame);
28-
} catch (StrictErrorException e) {
29-
// Intercept the strict exception to get a more localized expr ID for error reporting purposes
28+
} catch (LocalizedEvaluationException e) {
29+
// Intercept the localized exception to get a more specific expr ID for error reporting
3030
// Example: foo [1] && strict_err [2] -> ID 2 is propagated.
3131
return ErrorValue.create(e.exprId(), e);
3232
} catch (Exception e) {
@@ -39,9 +39,11 @@ static Object evalStrictly(
3939
try {
4040
return interpretable.eval(resolver, frame);
4141
} catch (CelRuntimeException e) {
42-
throw new StrictErrorException(e, interpretable.exprId());
42+
// Wrap with current interpretable's location
43+
throw new LocalizedEvaluationException(e, interpretable.exprId());
4344
} catch (Exception e) {
44-
throw new StrictErrorException(
45+
// Wrap generic exceptions with location
46+
throw new LocalizedEvaluationException(
4547
e.getCause(), CelErrorCode.INTERNAL_ERROR, interpretable.exprId());
4648
}
4749
}

runtime/src/main/java/dev/cel/runtime/planner/StrictErrorException.java renamed to runtime/src/main/java/dev/cel/runtime/planner/LocalizedEvaluationException.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 Google LLC
1+
// Copyright 2026 Google LLC
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -18,24 +18,28 @@
1818
import dev.cel.common.exceptions.CelRuntimeException;
1919

2020
/**
21-
* An exception that's raised when a strict call failed to invoke, which includes the source of
22-
* expression ID, along with canonical CelErrorCode.
21+
* Wraps a {@link CelRuntimeException} with its source expression ID for error reporting.
2322
*
24-
* <p>Note that StrictErrorException should not be surfaced directly back to the user.
23+
* <p>This is the ONLY exception type that propagates through evaluation in the planner. All
24+
* CelRuntimeExceptions from runtime helpers are immediately wrapped with location information to
25+
* track where the error occurred in the expression tree.
26+
*
27+
* <p>Note: This exception should not be surfaced directly to users - it's unwrapped in {@link
28+
* PlannedProgram}.
2529
*/
26-
final class StrictErrorException extends CelRuntimeException {
30+
final class LocalizedEvaluationException extends CelRuntimeException {
2731

2832
private final long exprId;
2933

3034
long exprId() {
3135
return exprId;
3236
}
3337

34-
StrictErrorException(CelRuntimeException cause, long exprId) {
38+
LocalizedEvaluationException(CelRuntimeException cause, long exprId) {
3539
this(cause, cause.getErrorCode(), exprId);
3640
}
3741

38-
StrictErrorException(Throwable cause, CelErrorCode errorCode, long exprId) {
42+
LocalizedEvaluationException(Throwable cause, CelErrorCode errorCode, long exprId) {
3943
super(cause, errorCode);
4044
this.exprId = exprId;
4145
}

runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,12 @@ private Object evalOrThrow(
106106

107107
private CelEvaluationException newCelEvaluationException(long exprId, Exception e) {
108108
CelEvaluationExceptionBuilder builder;
109-
if (e instanceof StrictErrorException) {
110-
// Preserve detailed error, including error codes if one exists.
111-
builder = CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) e.getCause());
109+
if (e instanceof LocalizedEvaluationException) {
110+
// Use the localized expr ID (most specific error location)
111+
LocalizedEvaluationException localized = (LocalizedEvaluationException) e;
112+
exprId = localized.exprId();
113+
builder =
114+
CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) localized.getCause());
112115
} else if (e instanceof CelRuntimeException) {
113116
builder = CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) e);
114117
} else {

runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public Program plan(CelAbstractSyntaxTree ast) throws CelEvaluationException {
9191
private PlannedInterpretable plan(CelExpr celExpr, PlannerContext ctx) {
9292
switch (celExpr.getKind()) {
9393
case CONSTANT:
94-
return planConstant(celExpr.constant());
94+
return planConstant(celExpr.id(), celExpr.constant());
9595
case IDENT:
9696
return planIdent(celExpr, ctx);
9797
case SELECT:
@@ -134,22 +134,22 @@ private PlannedInterpretable planSelect(CelExpr celExpr, PlannerContext ctx) {
134134
return attribute.addQualifier(celExpr.id(), qualifier);
135135
}
136136

137-
private PlannedInterpretable planConstant(CelConstant celConstant) {
137+
private PlannedInterpretable planConstant(long exprId, CelConstant celConstant) {
138138
switch (celConstant.getKind()) {
139139
case NULL_VALUE:
140-
return EvalConstant.create(celConstant.nullValue());
140+
return EvalConstant.create(exprId, celConstant.nullValue());
141141
case BOOLEAN_VALUE:
142-
return EvalConstant.create(celConstant.booleanValue());
142+
return EvalConstant.create(exprId, celConstant.booleanValue());
143143
case INT64_VALUE:
144-
return EvalConstant.create(celConstant.int64Value());
144+
return EvalConstant.create(exprId, celConstant.int64Value());
145145
case UINT64_VALUE:
146-
return EvalConstant.create(celConstant.uint64Value());
146+
return EvalConstant.create(exprId, celConstant.uint64Value());
147147
case DOUBLE_VALUE:
148-
return EvalConstant.create(celConstant.doubleValue());
148+
return EvalConstant.create(exprId, celConstant.doubleValue());
149149
case STRING_VALUE:
150-
return EvalConstant.create(celConstant.stringValue());
150+
return EvalConstant.create(exprId, celConstant.stringValue());
151151
case BYTES_VALUE:
152-
return EvalConstant.create(celConstant.bytesValue());
152+
return EvalConstant.create(exprId, celConstant.bytesValue());
153153
default:
154154
throw new IllegalStateException("Unsupported kind: " + celConstant.getKind());
155155
}
@@ -168,7 +168,7 @@ private PlannedInterpretable planIdent(CelExpr celExpr, PlannerContext ctx) {
168168
private PlannedInterpretable planCheckedIdent(
169169
long id, CelReference identRef, ImmutableMap<Long, CelType> typeMap) {
170170
if (identRef.value().isPresent()) {
171-
return planConstant(identRef.value().get());
171+
return planConstant(id, identRef.value().get());
172172
}
173173

174174
CelType type = typeMap.get(id);
@@ -181,7 +181,7 @@ private PlannedInterpretable planCheckedIdent(
181181
() ->
182182
new NoSuchElementException(
183183
"Reference to an undefined type: " + identRef.name()));
184-
return EvalConstant.create(identType);
184+
return EvalConstant.create(id, identType);
185185
}
186186

187187
return EvalAttribute.create(id, attributeFactory.newAbsoluteAttribute(identRef.name()));

0 commit comments

Comments
 (0)