Skip to content

Commit 8ccfb0b

Browse files
committed
Catch exception variable naming
1 parent 38ee182 commit 8ccfb0b

37 files changed

Lines changed: 213 additions & 177 deletions

File tree

.github/agents/knowledge/general-rules.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
1717
| Style | Style guide | Always ||
1818
| Style | Uppercase field names should reflect semantic constants, not simply `static final` | Always ||
1919
| Naming | Getter naming (`get` / `is`) | Always ||
20+
| Style | Prefer `e` for used exceptions, prefer `f` for used exceptions in nested catch clauses when an outer catch already uses `e`, allow `error` for specific `*Error` catch types, prefer `t` for used `Throwable` values, and `ignored` for intentionally unused catch parameters | Catch clauses, `Throwable` params ||
2021
| Naming | Module/package naming | New or renamed modules/packages | `module-naming.md` |
2122
| Javaagent | Advice patterns | `@Advice` classes | `javaagent-advice-patterns.md` |
2223
| Javaagent | Module structure patterns | `InstrumentationModule`, `TypeInstrumentation`, `Singletons` | `javaagent-module-patterns.md` |
@@ -54,6 +55,21 @@ Flag real defects, including:
5455

5556
Only flag substantive problems, not stylistic preference.
5657

58+
## [Javaagent] Best-Effort Suppressed Failures
59+
60+
When javaagent runtime code intentionally suppresses a `Throwable` to avoid breaking the
61+
application, do not silently swallow it.
62+
63+
- Prefer `ExceptionLogger.logSuppressedError(...)` for suppressed failures in javaagent code.
64+
It logs at `FINE` and matches the agent's default ByteBuddy advice suppression path in
65+
`ExceptionHandlers.defaultExceptionHandler()`.
66+
- Use this for best-effort hooks such as response customizers, bootstrap fallbacks, or other
67+
optional extension points where failure must not change application behavior.
68+
- Do not introduce ad-hoc local loggers for one-off suppressed javaagent failures when
69+
`ExceptionLogger` is available from bootstrap.
70+
- Keep the message action-oriented and specific (for example, "Failed to customize Netty 4.1
71+
HTTP server response").
72+
5773
## [Style] Style Guide
5874

5975
Read and apply `docs/contributing/style-guide.md`.
@@ -76,6 +92,21 @@ Do not flag the following patterns (common false positives):
7692

7793
Public API getters should use `get*` (or `is*` for booleans).
7894

95+
## [Style] Catch Exception Variable Naming
96+
97+
Prefer `e` as the exception variable name in catch clauses when the exception is
98+
used.
99+
100+
For nested catch clauses, when an outer catch already uses `e`, prefer `f` as the
101+
inner exception variable name when the exception is used.
102+
103+
`error` is also acceptable when the caught type is a specific `*Error` subtype.
104+
105+
Prefer `t` for used `Throwable` values, including `catch (Throwable t)` and
106+
`Throwable` callback parameters.
107+
108+
If a catch parameter is intentionally unused, prefer `ignored` over `ignore`.
109+
79110
## [Style] Prefer Instance Creation Over Singletons
80111

81112
Stateless implementations of telemetry interfaces — `TextMapGetter`, `TextMapSetter`,

instrumentation/grails-3.0/javaagent/src/test/java/test/GrailsTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ private static String getContextPathKey() {
8686
try {
8787
ServerProperties.class.getDeclaredMethod("getServlet");
8888
return "server.servlet.contextPath";
89-
} catch (NoSuchMethodException ignore) {
89+
} catch (NoSuchMethodException ignored) {
9090
return "server.context-path";
9191
}
9292
}

instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/BodySizeUtil.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@ final class BodySizeUtil {
1818
private static Class<?> getMessageLiteClass() {
1919
try {
2020
return Class.forName("com.google.protobuf.MessageLite");
21-
} catch (Exception ignore) {
21+
} catch (Exception ignored) {
2222
return null;
2323
}
2424
}
2525

2626
private static Method getSerializedSizeMethod(Class<?> clazz) {
2727
try {
2828
return clazz.getMethod("getSerializedSize");
29-
} catch (NoSuchMethodException ignore) {
29+
} catch (NoSuchMethodException ignored) {
3030
return null;
3131
}
3232
}
@@ -40,7 +40,7 @@ static <T> Long getBodySize(T message) {
4040
}
4141
try {
4242
return ((Integer) serializedSizeMethod.invoke(message)).longValue();
43-
} catch (Throwable ignore) {
43+
} catch (Throwable ignored) {
4444
return null;
4545
}
4646
}

instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcStreamingTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ private static void usePlainText(ManagedChannelBuilder<?> channelBuilder) throws
497497
.getClass()
498498
.getMethod("usePlaintext", boolean.class)
499499
.invoke(channelBuilder, true);
500-
} catch (NoSuchMethodException unused) {
500+
} catch (NoSuchMethodException ignored) {
501501
channelBuilder.getClass().getMethod("usePlaintext").invoke(channelBuilder);
502502
}
503503
}

instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1703,7 +1703,7 @@ private static void usePlainText(ManagedChannelBuilder<?> channelBuilder) throws
17031703
.getClass()
17041704
.getMethod("usePlaintext", boolean.class)
17051705
.invoke(channelBuilder, true);
1706-
} catch (NoSuchMethodException unused) {
1706+
} catch (NoSuchMethodException ignored) {
17071707
channelBuilder.getClass().getMethod("usePlaintext").invoke(channelBuilder);
17081708
}
17091709
}

instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/SoapFaultBuilderInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public static void onExit(@Advice.Argument(2) Throwable throwable) {
4545
// if fiber is not attached current() throws IllegalStateException
4646
try {
4747
request = Fiber.current().getPacket();
48-
} catch (IllegalStateException ignore) {
48+
} catch (IllegalStateException ignored) {
4949
// fiber not available
5050
}
5151
if (request != null) {

instrumentation/jaxws/jaxws-metro-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/metro/TracingTube.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public NextAction processException(Throwable throwable) {
5454
// if fiber is not attached current() throws IllegalStateException
5555
try {
5656
request = Fiber.current().getPacket();
57-
} catch (IllegalStateException ignore) {
57+
} catch (IllegalStateException ignored) {
5858
// fiber not available
5959
}
6060
if (request != null) {

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public static DbInfo computeDbInfo(Connection connection) {
107107
if (url != null) {
108108
try {
109109
return JdbcConnectionUrlParser.parse(url, connection.getClientInfo());
110-
} catch (Throwable ex) {
110+
} catch (Throwable ignored) {
111111
// getClientInfo is likely not allowed.
112112
return JdbcConnectionUrlParser.parse(url, null);
113113
}

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,8 @@ static void tearDown() {
140140
if (dataSource instanceof Closeable) {
141141
try {
142142
((Closeable) dataSource).close();
143-
} catch (IOException ignore) {
144-
// ignore
143+
} catch (IOException ignored) {
144+
// ignore exceptions during close
145145
}
146146
}
147147
}));

instrumentation/jetty/jetty-8.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/QueuedThreadPoolTest.java

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
package io.opentelemetry.javaagent.instrumentation.jetty.v8_0;
77

8-
import static org.junit.jupiter.api.Assumptions.assumeTrue;
8+
import static org.junit.jupiter.api.Assumptions.abort;
99

1010
import io.opentelemetry.api.trace.SpanKind;
1111
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
@@ -25,29 +25,26 @@ void dispatchPropagates() throws Exception {
2525
QueuedThreadPool pool = new QueuedThreadPool();
2626
// run test only if QueuedThreadPool has dispatch method
2727
// dispatch method was removed in jetty 9.1
28-
Method dispatch = null;
28+
Method dispatch;
2929
try {
3030
dispatch = QueuedThreadPool.class.getMethod("dispatch", Runnable.class);
31-
} catch (NoSuchMethodException ignore) {
32-
// ignore
31+
} catch (NoSuchMethodException ignored) {
32+
abort("Jetty 9.1+ removed dispatch(Runnable)");
33+
return;
3334
}
34-
assumeTrue(dispatch != null);
3535
pool.start();
3636

37-
Method finalDispatch = dispatch;
3837
testing.runWithSpan(
3938
"parent",
4039
() -> {
4140
// this child will have a span
4241
JavaAsyncChild child1 = new JavaAsyncChild();
4342
// this child won't
4443
JavaAsyncChild child2 = new JavaAsyncChild(false, false);
45-
if (finalDispatch != null) {
46-
finalDispatch.invoke(pool, child1);
47-
finalDispatch.invoke(pool, child2);
48-
child1.waitForCompletion();
49-
child2.waitForCompletion();
50-
}
44+
dispatch.invoke(pool, child1);
45+
dispatch.invoke(pool, child2);
46+
child1.waitForCompletion();
47+
child2.waitForCompletion();
5148
});
5249

5350
testing.waitAndAssertTraces(
@@ -67,23 +64,20 @@ void dispatchPropagatesLambda() throws Exception {
6764
QueuedThreadPool pool = new QueuedThreadPool();
6865
// run test only if QueuedThreadPool has dispatch method
6966
// dispatch method was removed in jetty 9.1
70-
Method dispatch = null;
67+
Method dispatch;
7168
try {
7269
dispatch = QueuedThreadPool.class.getMethod("dispatch", Runnable.class);
73-
} catch (NoSuchMethodException ignore) {
74-
// ignore
70+
} catch (NoSuchMethodException ignored) {
71+
abort("Jetty 9.1+ removed dispatch(Runnable)");
72+
return;
7573
}
76-
assumeTrue(dispatch != null);
7774
pool.start();
7875

7976
JavaAsyncChild child = new JavaAsyncChild(true, true);
80-
Method finalDispatch = dispatch;
8177
testing.runWithSpan(
8278
"parent",
8379
() -> {
84-
if (finalDispatch != null) {
85-
finalDispatch.invoke(pool, JavaLambdaMaker.lambda(child));
86-
}
80+
dispatch.invoke(pool, JavaLambdaMaker.lambda(child));
8781
});
8882

8983
// We block in child to make sure spans close in predictable order

0 commit comments

Comments
 (0)