Skip to content

Commit ccdefe9

Browse files
torosentCopilot
andcommitted
Fix span lifecycle issues found in code review
- Activity worker: track error separately, end span once in finally block (fixes double span.end() call on error path) - OrchestrationRunner: close scope before ending span, add null check (fixes scope/span lifecycle ordering and potential NPE) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 183cb07 commit ccdefe9

File tree

2 files changed

+7
-6
lines changed

2 files changed

+7
-6
lines changed

client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcWorker.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,22 +286,22 @@ public void startAndBlock() {
286286
// TODO: Run this on a worker pool thread: https://www.baeldung.com/thread-pool-java-and-guava
287287
String output = null;
288288
TaskFailureDetails failureDetails = null;
289+
Throwable activityError = null;
289290
try {
290291
output = taskActivityExecutor.execute(
291292
activityRequest.getName(),
292293
activityRequest.getInput().getValue(),
293294
activityRequest.getTaskId());
294295
} catch (Throwable e) {
295-
TracingHelper.endSpan(activitySpan, e);
296-
activitySpan = null;
296+
activityError = e;
297297
failureDetails = TaskFailureDetails.newBuilder()
298298
.setErrorType(e.getClass().getName())
299299
.setErrorMessage(e.getMessage())
300300
.setStackTrace(StringValue.of(FailureDetails.getFullStackTrace(e)))
301301
.build();
302302
} finally {
303303
activityScope.close();
304-
TracingHelper.endSpan(activitySpan, null);
304+
TracingHelper.endSpan(activitySpan, activityError);
305305
}
306306

307307
ActivityResponse.Builder responseBuilder = ActivityResponse.newBuilder()

client/src/main/java/com/microsoft/durabletask/OrchestrationRunner.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,17 +177,18 @@ public TaskOrchestration create() {
177177

178178
// TODO: Error handling
179179
TaskOrchestratorResult taskOrchestratorResult;
180+
Throwable orchestrationError = null;
180181
try {
181182
taskOrchestratorResult = taskOrchestrationExecutor.execute(
182183
orchestratorRequest.getPastEventsList(),
183184
orchestratorRequest.getNewEventsList());
184185
} catch (Exception e) {
185-
TracingHelper.endSpan(orchestrationSpan, e);
186+
orchestrationError = e;
186187
throw e instanceof RuntimeException ? (RuntimeException) e : new RuntimeException(e);
187188
} finally {
188-
orchestrationScope.close();
189+
if (orchestrationScope != null) orchestrationScope.close();
190+
TracingHelper.endSpan(orchestrationSpan, orchestrationError);
189191
}
190-
TracingHelper.endSpan(orchestrationSpan, null);
191192

192193
OrchestratorService.OrchestratorResponse response = OrchestratorService.OrchestratorResponse.newBuilder()
193194
.setInstanceId(orchestratorRequest.getInstanceId())

0 commit comments

Comments
 (0)