From b47fd7109ba36b363f3ea6445abddc10b1bed8ef Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Fri, 26 Sep 2025 19:18:53 +0200 Subject: [PATCH] refactor: replace assertions with exceptions where possible Assertions should enforce invariants, not signal error conditions. Unlike Exceptions, AssertionErrors are not caught by catch (Exception e) and may be more unpredictable. We should try to only ever use assertions for cases that can never happen unless there's been a programming mistake on our side. Everything else, e.g. server responses, network errors, client's input (ORM) should be dealt with via exceptions. --- .../client6/v1/api/collections/Vectors.java | 4 +++- .../api/collections/aggregate/AggregateRequest.java | 6 +++--- .../v1/api/collections/pagination/AsyncPage.java | 4 +++- .../collections/pagination/CursorSpliterator.java | 4 +++- .../v1/api/collections/query/QueryRequest.java | 2 +- .../client6/v1/internal/orm/PojoDescriptor.java | 2 +- .../client6/v1/internal/orm/PojoReader.java | 13 ++++++++----- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/weaviate/client6/v1/api/collections/Vectors.java b/src/main/java/io/weaviate/client6/v1/api/collections/Vectors.java index 2c5dacf4b..b89cf7fcb 100644 --- a/src/main/java/io/weaviate/client6/v1/api/collections/Vectors.java +++ b/src/main/java/io/weaviate/client6/v1/api/collections/Vectors.java @@ -186,7 +186,9 @@ public Vectors read(JsonReader in) throws IOException { vector = float_1d.fromJsonTree(array); } - assert (vector instanceof float[]) || (vector instanceof float[][]) : "invalid vector type"; + assert (vector instanceof float[]) || (vector instanceof float[][]) + : "invalid vector type " + vector.getClass(); + namedVectors.put(vectorName, vector); } } diff --git a/src/main/java/io/weaviate/client6/v1/api/collections/aggregate/AggregateRequest.java b/src/main/java/io/weaviate/client6/v1/api/collections/aggregate/AggregateRequest.java index 89e891cf1..fa8290f64 100644 --- a/src/main/java/io/weaviate/client6/v1/api/collections/aggregate/AggregateRequest.java +++ b/src/main/java/io/weaviate/client6/v1/api/collections/aggregate/AggregateRequest.java @@ -4,8 +4,8 @@ import java.util.HashMap; import java.util.Map; -import io.weaviate.client6.v1.internal.DateUtil; import io.weaviate.client6.v1.api.collections.CollectionHandleDefaults; +import io.weaviate.client6.v1.internal.DateUtil; import io.weaviate.client6.v1.internal.grpc.Rpc; import io.weaviate.client6.v1.internal.grpc.protocol.WeaviateGrpc.WeaviateBlockingStub; import io.weaviate.client6.v1.internal.grpc.protocol.WeaviateGrpc.WeaviateFutureStub; @@ -79,7 +79,7 @@ static Rpc(property, groupBy.getBooleans().getValuesList().toArray(Boolean[]::new)); } else { - assert false : "(aggregate) branch not covered"; + throw new IllegalArgumentException(property + " data type is not supported"); } var properties = unmarshalAggregation(result.getAggregations()); @@ -148,7 +148,7 @@ private static Map unmarshalAggregation(WeaviateProtoAggregate.A metric.hasMode() ? metric.getMode() : null, metric.hasSum() ? metric.getSum() : null); } else { - assert false : "branch not covered"; + throw new IllegalArgumentException(property + " data type is not supported"); } if (value != null) { diff --git a/src/main/java/io/weaviate/client6/v1/api/collections/pagination/AsyncPage.java b/src/main/java/io/weaviate/client6/v1/api/collections/pagination/AsyncPage.java index baddef6f1..691b8bad6 100644 --- a/src/main/java/io/weaviate/client6/v1/api/collections/pagination/AsyncPage.java +++ b/src/main/java/io/weaviate/client6/v1/api/collections/pagination/AsyncPage.java @@ -61,7 +61,9 @@ public CompletableFuture> fetchNextPage() { // If it is null after the first iteration it is // because we haven't requested Metadata.UUID, in which // case pagination will continue to run unbounded. - assert nextCursor != null : "page cursor is null"; + if (nextCursor == null) { + throw new IllegalStateException("page cursor is null"); + } return new AsyncPage<>(nextCursor, pageSize, fetch, nextPage); }); } diff --git a/src/main/java/io/weaviate/client6/v1/api/collections/pagination/CursorSpliterator.java b/src/main/java/io/weaviate/client6/v1/api/collections/pagination/CursorSpliterator.java index 27132fc2e..3b0335d88 100644 --- a/src/main/java/io/weaviate/client6/v1/api/collections/pagination/CursorSpliterator.java +++ b/src/main/java/io/weaviate/client6/v1/api/collections/pagination/CursorSpliterator.java @@ -45,7 +45,9 @@ public boolean tryAdvance(Consumer void setProperty(String property, WeaviateProtoProperties.Val builder.setOffsetDateTimeArray(property, dates); } } else { - assert false : "(query) branch not covered"; + throw new IllegalArgumentException(property + " data type is not supported"); } } } diff --git a/src/main/java/io/weaviate/client6/v1/internal/orm/PojoDescriptor.java b/src/main/java/io/weaviate/client6/v1/internal/orm/PojoDescriptor.java index bcb1b4703..d0a1f218b 100644 --- a/src/main/java/io/weaviate/client6/v1/internal/orm/PojoDescriptor.java +++ b/src/main/java/io/weaviate/client6/v1/internal/orm/PojoDescriptor.java @@ -147,7 +147,7 @@ private ObjectBuilder inspectClass(CollectionConfig.Builder b) } if (ctor == null) { - throw new IllegalArgumentException(type.getCanonicalName() + " fields are not supported"); + throw new IllegalArgumentException(type.getCanonicalName() + " property is not supported"); } assert ctor != null; diff --git a/src/main/java/io/weaviate/client6/v1/internal/orm/PojoReader.java b/src/main/java/io/weaviate/client6/v1/internal/orm/PojoReader.java index 82ccea5a3..41ace15d0 100644 --- a/src/main/java/io/weaviate/client6/v1/internal/orm/PojoReader.java +++ b/src/main/java/io/weaviate/client6/v1/internal/orm/PojoReader.java @@ -15,12 +15,15 @@ public Map readProperties() { var out = new HashMap(); for (var field : properties.getClass().getDeclaredFields()) { var propertyName = PojoDescriptor.propertyName(field); - field.setAccessible(true); - try { - out.put(propertyName, field.get(properties)); - } catch (IllegalAccessException e) { - assert e == null : e.getMessage(); + if (field.trySetAccessible()) { + try { + out.put(propertyName, field.get(properties)); + } catch (IllegalAccessException e) { + new RuntimeException("accessible flag set but access denied", e); + } } + // TODO: how do we handle the case where a property is not accessible? + // E.g. we weren't able to set `accessible` flag. } return out; }