Skip to content

Commit 986dbaa

Browse files
authored
Ensure Serializer runtime exceptions are rethrown as IOException (#6969)
1 parent 98c3fc1 commit 986dbaa

File tree

2 files changed

+145
-5
lines changed

2 files changed

+145
-5
lines changed

exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Serializer.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import io.opentelemetry.api.common.Attributes;
1010
import io.opentelemetry.sdk.internal.DynamicPrimitiveLongList;
1111
import java.io.IOException;
12+
import java.io.UncheckedIOException;
1213
import java.nio.ByteBuffer;
1314
import java.util.Collection;
1415
import java.util.List;
@@ -535,7 +536,11 @@ public <T> void serializeRepeatedMessageWithContext(
535536
if (!messages.isEmpty()) {
536537
RepeatedElementWriter<T> writer = context.getInstance(key, RepeatedElementWriter::new);
537538
writer.initialize(field, this, marshaler, context);
538-
messages.forEach(writer);
539+
try {
540+
messages.forEach(writer);
541+
} catch (UncheckedIOException e) {
542+
throw e.getCause();
543+
}
539544
}
540545

541546
writeEndRepeated();
@@ -559,7 +564,11 @@ public <K, V> void serializeRepeatedMessageWithContext(
559564
RepeatedElementPairWriter<K, V> writer =
560565
context.getInstance(key, RepeatedElementPairWriter::new);
561566
writer.initialize(field, this, marshaler, context);
562-
messages.forEach(writer);
567+
try {
568+
messages.forEach(writer);
569+
} catch (UncheckedIOException e) {
570+
throw e.getCause();
571+
}
563572
}
564573

565574
writeEndRepeated();
@@ -582,7 +591,11 @@ public void serializeRepeatedMessageWithContext(
582591
RepeatedElementPairWriter<AttributeKey<?>, Object> writer =
583592
context.getInstance(ATTRIBUTES_WRITER_KEY, RepeatedElementPairWriter::new);
584593
writer.initialize(field, this, marshaler, context);
585-
attributes.forEach(writer);
594+
try {
595+
attributes.forEach(writer);
596+
} catch (UncheckedIOException e) {
597+
throw e.getCause();
598+
}
586599
}
587600

588601
writeEndRepeated();
@@ -619,7 +632,7 @@ public void accept(T element) {
619632
marshaler.writeTo(output, element, context);
620633
output.writeEndRepeatedElement();
621634
} catch (IOException e) {
622-
throw new IllegalStateException(e);
635+
throw new UncheckedIOException(e);
623636
}
624637
}
625638
}
@@ -655,7 +668,7 @@ public void accept(K key, V value) {
655668
marshaler.writeTo(output, key, value, context);
656669
output.writeEndRepeatedElement();
657670
} catch (IOException e) {
658-
throw new IllegalStateException(e);
671+
throw new UncheckedIOException(e);
659672
}
660673
}
661674
}

exporters/common/src/test/java/io/opentelemetry/exporter/internal/marshal/MarshalerTest.java

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,17 @@
1111
import static org.mockito.Mockito.doThrow;
1212
import static org.mockito.Mockito.mock;
1313

14+
import io.opentelemetry.api.common.AttributeKey;
15+
import io.opentelemetry.api.common.Attributes;
16+
import java.io.ByteArrayOutputStream;
1417
import java.io.IOException;
1518
import java.io.OutputStream;
19+
import java.util.Collections;
20+
import java.util.stream.Stream;
1621
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.Arguments;
24+
import org.junit.jupiter.params.provider.MethodSource;
1725

1826
class MarshalerTest {
1927

@@ -47,4 +55,123 @@ protected void writeTo(Serializer output) throws IOException {
4755
assertThatThrownBy(() -> marshaler.writeBinaryTo(os)).isInstanceOf(IOException.class);
4856
assertThatThrownBy(() -> marshaler.writeJsonTo(os)).isInstanceOf(IOException.class);
4957
}
58+
59+
/**
60+
* This test ensures that instances where serializer produces runtime exceptions are properly
61+
* converted back to checked {@link IOException}.
62+
*
63+
* <p>At various points in {@link Serializer}, we use {@code .forEach}-style methods which accept
64+
* {@link java.util.function.Consumer} and {@link java.util.function.BiConsumer}. These consumers
65+
* that any checked exceptions are caught and rethrown as runtime exceptions. Its essential that
66+
* these runtime exceptions are re-converted back to checked {@link IOException} because calling
67+
* code's error handling is built on top of {@link IOException}. Failure to convert to {@link
68+
* IOException} leads to issues like this: <a
69+
* href="https://github.com/open-telemetry/opentelemetry-java/issues/6946">#6946</a>.
70+
*/
71+
@ParameterizedTest
72+
@MethodSource("writeToArgs")
73+
void writeTo_NoRuntimeExceptions(Writer writer) throws IOException {
74+
Marshaler marshaler =
75+
new Marshaler() {
76+
@Override
77+
public int getBinarySerializedSize() {
78+
return 0;
79+
}
80+
81+
@Override
82+
protected void writeTo(Serializer output) throws IOException {
83+
writer.writeTo(output);
84+
}
85+
};
86+
87+
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
88+
assertThatThrownBy(() -> marshaler.writeBinaryTo(baos)).isInstanceOf(IOException.class);
89+
}
90+
}
91+
92+
private static Stream<Arguments> writeToArgs() {
93+
ProtoFieldInfo fieldInfo = ProtoFieldInfo.create(0, 0, "name");
94+
MarshalerContext context = new MarshalerContext();
95+
ThrowingMarshaler<String> throwingMarshaler = new ThrowingMarshaler<>(new IOException("error"));
96+
ThrowingMarshaler2<String, String> throwingMarshaler2 =
97+
new ThrowingMarshaler2<>(new IOException("error"));
98+
ThrowingMarshaler2<AttributeKey<?>, Object> throwingMarshaler2Attributes =
99+
new ThrowingMarshaler2<>(new IOException("error"));
100+
101+
return Stream.of(
102+
Arguments.of(
103+
asWriter(
104+
output ->
105+
output.serializeRepeatedMessageWithContext(
106+
fieldInfo,
107+
Collections.singleton("value"),
108+
throwingMarshaler,
109+
context,
110+
MarshalerContext.key()))),
111+
Arguments.of(
112+
asWriter(
113+
output ->
114+
output.serializeRepeatedMessageWithContext(
115+
fieldInfo,
116+
Collections.singletonMap("key", "value"),
117+
throwingMarshaler2,
118+
context,
119+
MarshalerContext.key()))),
120+
Arguments.of(
121+
asWriter(
122+
output ->
123+
output.serializeRepeatedMessageWithContext(
124+
fieldInfo,
125+
Attributes.builder().put("key", "value").build(),
126+
throwingMarshaler2Attributes,
127+
context))));
128+
}
129+
130+
private static Writer asWriter(Writer writer) {
131+
return writer;
132+
}
133+
134+
@FunctionalInterface
135+
private interface Writer {
136+
void writeTo(Serializer output) throws IOException;
137+
}
138+
139+
private static class ThrowingMarshaler<T> implements StatelessMarshaler<T> {
140+
141+
private final IOException exception;
142+
143+
private ThrowingMarshaler(IOException exception) {
144+
this.exception = exception;
145+
}
146+
147+
@Override
148+
public int getBinarySerializedSize(T value, MarshalerContext context) {
149+
return 0;
150+
}
151+
152+
@Override
153+
public void writeTo(Serializer output, T value, MarshalerContext context) throws IOException {
154+
throw exception;
155+
}
156+
}
157+
158+
private static class ThrowingMarshaler2<K, V> implements StatelessMarshaler2<K, V> {
159+
160+
private final IOException exception;
161+
162+
private ThrowingMarshaler2(IOException exception) {
163+
this.exception = exception;
164+
}
165+
166+
@Override
167+
public int getBinarySerializedSize(K key, V value, MarshalerContext context) {
168+
return 0;
169+
}
170+
171+
@Override
172+
public void writeTo(Serializer output, K key, V value, MarshalerContext context)
173+
throws IOException {
174+
throw exception;
175+
}
176+
}
50177
}

0 commit comments

Comments
 (0)