From 0d5222f83afd821299952541e4a881241d86aed1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 12:39:55 +0000 Subject: [PATCH] fix(serve): stop logging InterruptedException as a 500 query error Production logs showed "[Error] error computing impacted targets" with a java.lang.InterruptedException stack out of BuildGraphHasher's runBlocking. That interrupt is not an application failure: it comes from stop()'s computeExecutor.shutdownNow() tearing down an in-flight request during JVM shutdown (HttpServer.stop's grace period is shorter than a hash generation), or from a timed-out request's future.cancel(true) leaving a stale interrupt flag on a pooled compute thread that then poisons an unrelated request's first blocking call. - handleQuery now maps InterruptedException to a 503 with a warn-level log instead of the generic 500 + error-level stack trace. - computeWithTimeout clears any leaked interrupt flag before running a task, so a cancelled request that never observed its interrupt (e.g. blocked in `synchronized` on the generation lock) cannot fail the next request served by the same pool thread. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014uD7fwXvjPfYipBf7pnhP5 --- .../com/bazel_diff/server/BazelDiffServer.kt | 19 ++- .../bazel_diff/server/BazelDiffServerTest.kt | 121 ++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) diff --git a/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt b/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt index 239c219..c3658a5 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/BazelDiffServer.kt @@ -223,6 +223,13 @@ class BazelDiffServer( } catch (e: GitClientException) { logger.e(e) { "git error computing impacted targets" } respondJson(exchange, 400, mapOf("error" to "git error: ${e.message}")) + } catch (e: InterruptedException) { + // The compute was interrupted, not broken: in practice [stop]'s shutdownNow() tearing + // down the executor while this request was mid-hash-generation (the JVM shutdown hook + // gives in-flight exchanges only a short grace period). Not an application error, so + // no stack-trace error log; 503 tells the load balancer to retry on another instance. + logger.w { "request interrupted mid-compute (server shutting down), abandoning" } + respondJson(exchange, 503, mapOf("error" to "request interrupted (server shutting down)")) } catch (e: Exception) { logger.e(e) { "error computing impacted targets" } respondJson(exchange, 500, mapOf("error" to (e.message ?: e.javaClass.simpleName))) @@ -239,7 +246,17 @@ class BazelDiffServer( */ private fun computeWithTimeout(compute: () -> T): T { if (requestTimeoutSeconds <= 0) return compute() - val future = computeExecutor.submit(Callable { compute() }) + val future = + computeExecutor.submit( + Callable { + // A timed-out request's cancel(true) can leave a stale interrupt flag on this pooled + // thread: a task blocked in an uninterruptible wait (e.g. `synchronized` on the hash + // generation lock) never observes the interrupt, so the flag survives the task and + // would make an unrelated request's first blocking call throw InterruptedException. + // Clear it before starting fresh work. + Thread.interrupted() + compute() + }) try { return future.get(requestTimeoutSeconds, TimeUnit.SECONDS) } catch (e: TimeoutException) { diff --git a/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt index 0efbc5d..346f68e 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/BazelDiffServerTest.kt @@ -20,6 +20,7 @@ import java.net.HttpURLConnection import java.net.URL import java.nio.charset.StandardCharsets import java.nio.file.Path +import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean import org.junit.After import org.junit.Before @@ -467,6 +468,126 @@ class BazelDiffServerTest : KoinTest { } } + @Test + fun interruptedComputeReturns503NotA500() { + // An InterruptedException out of the compute is the server being torn down mid-request + // (stop()'s shutdownNow() interrupting the in-flight hash generation), not an application + // error: it must map to 503 so the caller retries elsewhere, never the generic 500. + provider = FixedProvider(error = InterruptedException("sleep interrupted")) + val response = get("/impacted_targets?from=a&to=b") + assertThat(response.code).isEqualTo(503) + assertThat(response.body).contains("interrupted") + } + + @Test + fun interruptedComputeOnTimeoutExecutorReturns503NotA500() { + // Same as above, but with a request timeout configured the compute runs on the executor and + // the InterruptedException arrives wrapped in an ExecutionException; the unwrap in + // computeWithTimeout must still land it in the 503 branch (this is the exact path of the + // production "[Error] error computing impacted targets" InterruptedException stack). + val interruptedProvider = + object : ImpactedTargetsProvider { + override fun getImpactedTargets( + fromRev: String, + toRev: String, + targetTypes: Set?, + modifiedFilepaths: Set, + profiler: QueryProfiler? + ): ImpactedTargetsResult = throw InterruptedException("interrupted by shutdownNow") + + override fun getImpactedTargetsWithDistances( + fromRev: String, + toRev: String, + targetTypes: Set?, + modifiedFilepaths: Set, + profiler: QueryProfiler? + ) = throw UnsupportedOperationException() + } + val timeoutServer = BazelDiffServer(0, interruptedProvider, requestTimeoutSeconds = 30) { true } + timeoutServer.start() + try { + val conn = + URL("http://localhost:${timeoutServer.boundPort()}/impacted_targets?from=a&to=b") + .openConnection() as HttpURLConnection + val code = conn.responseCode + val body = + (conn.errorStream ?: conn.inputStream)?.let { + BufferedReader(InputStreamReader(it, StandardCharsets.UTF_8)).readText() + } ?: "" + conn.disconnect() + assertThat(code).isEqualTo(503) + assertThat(body).contains("interrupted") + } finally { + timeoutServer.stop() + } + } + + @Test + fun staleInterruptFlagFromTimedOutRequestDoesNotPoisonNextRequest() { + // A timed-out request's future.cancel(true) interrupts its pooled compute thread; a task + // blocked in an uninterruptible wait (like `synchronized` on the hash generation lock) never + // observes that interrupt, so the flag can survive onto the pool thread and would make the + // next request's first blocking call throw InterruptedException. computeWithTimeout must + // start every task with a cleared interrupt flag. + val sawInterruptedFlag = AtomicBoolean(false) + val flagCheckingProvider = + object : ImpactedTargetsProvider { + override fun getImpactedTargets( + fromRev: String, + toRev: String, + targetTypes: Set?, + modifiedFilepaths: Set, + profiler: QueryProfiler? + ): ImpactedTargetsResult { + if (Thread.currentThread().isInterrupted) sawInterruptedFlag.set(true) + if (fromRev == "poison") { + // Uninterruptible wait past the 1s timeout: swallow the cancel(true) interrupt the + // way a `synchronized` block would, then leave the flag set on exit exactly as a + // real uninterruptible wait does. + val deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(1_500) + var interrupted = false + while (System.nanoTime() < deadline) { + try { + Thread.sleep(50) + } catch (e: InterruptedException) { + interrupted = true + } + } + if (interrupted) Thread.currentThread().interrupt() + } + return ImpactedTargetsResult(fromRev, toRev, emptyList()) + } + + override fun getImpactedTargetsWithDistances( + fromRev: String, + toRev: String, + targetTypes: Set?, + modifiedFilepaths: Set, + profiler: QueryProfiler? + ) = throw UnsupportedOperationException() + } + val timeoutServer = BazelDiffServer(0, flagCheckingProvider, requestTimeoutSeconds = 1) { true } + timeoutServer.start() + try { + val conn = + URL("http://localhost:${timeoutServer.boundPort()}/impacted_targets?from=poison&to=b") + .openConnection() as HttpURLConnection + assertThat(conn.responseCode).isEqualTo(504) + conn.disconnect() + // Give the abandoned poison task time to finish and return its thread to the pool. + Thread.sleep(1_000) + val conn2 = + URL("http://localhost:${timeoutServer.boundPort()}/impacted_targets?from=a&to=b") + .openConnection() as HttpURLConnection + val code = conn2.responseCode + conn2.disconnect() + assertThat(code).isEqualTo(200) + assertThat(sawInterruptedFlag.get()).isEqualTo(false) + } finally { + timeoutServer.stop() + } + } + @Test fun distancesUnavailableReturns400() { provider =