gemini-code-assist[bot] commented on code in PR #39124:
URL: https://github.com/apache/beam/pull/39124#discussion_r3481849065
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java:
##########
@@ -2105,9 +2105,10 @@ public static <T> void readSource(
// the same order.
BoundedSource.BoundedReader<T> reader =
streamSource.createReader(options);
+ T current = null;
try {
if (reader.start()) {
- outputReceiver.get(rowTag).output(reader.getCurrent());
+ current = reader.getCurrent();
Review Comment:
If `reader.getCurrent()` returns `null`, the current implementation will
silently skip outputting the element without throwing an exception or routing
it to error handling, whereas previously it would have thrown a
`NullPointerException` inside the `try` block. To preserve the original
behavior and avoid silent data loss, use `java.util.Objects.requireNonNull` to
ensure a `NullPointerException` is thrown and caught if the reader returns
`null`.
```suggestion
current = java.util.Objects.requireNonNull(reader.getCurrent(),
"Reader returned null element");
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.java:
##########
@@ -2120,11 +2121,15 @@ public static <T> void readSource(
(Exception) e.getCause(),
"Unable to parse record reading from BigQuery");
}
+ if (current != null) {
+ outputReceiver.get(rowTag).output(current);
+ }
while (true) {
+ current = null;
try {
if (reader.advance()) {
- outputReceiver.get(rowTag).output(reader.getCurrent());
+ current = reader.getCurrent();
Review Comment:
Similar to the `reader.start()` block, if `reader.getCurrent()` returns
`null` during advancement, it will be silently skipped. Use
`java.util.Objects.requireNonNull` to ensure any `null` elements are caught and
handled by the `try-catch` block.
```suggestion
current = java.util.Objects.requireNonNull(reader.getCurrent(),
"Reader returned null element");
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/FhirIO.java:
##########
@@ -602,8 +602,9 @@ public void instantiateHealthcareClient() throws
IOException {
@ProcessElement
public void processElement(ProcessContext context) {
String resourceId = context.element();
+ String resource = null;
try {
- context.output(fetchResource(this.client, resourceId));
+ resource = fetchResource(this.client, resourceId);
Review Comment:
If `fetchResource` returns `null`, the resource will be silently skipped
instead of being routed to the dead-letter queue as a failure (which would have
happened previously due to a `NullPointerException` when calling
`context.output(null)` inside the `try` block). Use
`java.util.Objects.requireNonNull` to preserve the original error-handling
behavior.
```suggestion
resource =
java.util.Objects.requireNonNull(fetchResource(this.client, resourceId),
"Fetched resource is null");
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2IO.java:
##########
@@ -365,11 +365,15 @@ public void instantiateHealthcareClient() throws
IOException {
@ProcessElement
public void processElement(ProcessContext context) {
String msgId = context.element();
+ HL7v2Message message = null;
try {
- context.output(client.fetchMessage(msgId));
+ message = client.fetchMessage(msgId);
Review Comment:
If `client.fetchMessage(msgId)` returns `null`, the message will be silently
skipped instead of being routed to the dead-letter queue. Use
`java.util.Objects.requireNonNull` to ensure a `NullPointerException` is thrown
and caught inside the `try` block, preserving the original dead-letter queue
routing.
```suggestion
message =
java.util.Objects.requireNonNull(client.fetchMessage(msgId), "Fetched message
is null");
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HL7v2IO.java:
##########
@@ -487,15 +491,18 @@ public void instantiateHealthcareClient() throws
IOException {
@ProcessElement
public void processElement(ProcessContext context) {
String msgId = context.element().getHl7v2MessageId();
+ HL7v2ReadResponse response = null;
try {
- HL7v2ReadResponse response =
+ response =
HL7v2ReadResponse.of(context.element().getMetadata(),
client.fetchMessage(msgId));
Review Comment:
If `HL7v2ReadResponse.of` returns `null`, the response will be silently
skipped instead of being routed to the dead-letter queue. Use
`java.util.Objects.requireNonNull` to ensure a `NullPointerException` is thrown
and caught inside the `try` block.
```java
response = java.util.Objects.requireNonNull(
HL7v2ReadResponse.of(context.element().getMetadata(),
client.fetchMessage(msgId)),
"Fetched response is null");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
