Stop swallowing CancellationException in suspend code#70
Merged
Conversation
Several broad `catch (e: Exception)` blocks and `runCatching {}` calls in
suspend functions absorbed CancellationException (which is an Exception, and
runCatching catches Throwable). This broke cooperative cancellation: when the
enclosing scope was cancelled (browser.stop(), withTimeout), affected coroutines
kept running CDP work or completed "successfully" instead of unwinding, and the
WebSocket receive loop printed a spurious stack trace on every clean shutdown.
Each affected site now rethrows CancellationException before the broad catch:
- DefaultConnection.prepareHeadless/prepareExpert (runCatching -> try/catch)
- DefaultConnection.startListening (inner emit + outer incoming-loop catches)
- DefaultTab.findElementsByText (resolveNode + element-build catches)
- DefaultBrowser.testConnection
- DefaultElement.mouseMoveWithTrajectory viewport query
Non-cancellation behavior is preserved (other exceptions stay swallowed/logged
as before). Sites that already rethrow were left unchanged.
Verified red->green with CancellationPropagationTest, which drives the real
findElementsByText through a stubbed callCommand: before the fix the cancellation
is swallowed and the call returns normally (test fails); after, it propagates.
Full :core:jvmTest (real Chrome) and :opentelemetry:jvmTest pass.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Several broad
catch (e: Exception)blocks andrunCatching {}calls in suspend functions were absorbingCancellationException. In KotlinCancellationExceptionis anException(andrunCatchingcatchesThrowable), so these blocks silently swallowed it and broke cooperative cancellation.Why it matters: when the enclosing scope is cancelled (
browser.stop()→coroutineScope.cancel(), or a caller'swithTimeout), the affected coroutines kept issuing CDP work / completed "successfully" instead of unwinding, and the WebSocket receive loop printed a spurious stack trace on every clean shutdown — classic "won't die" / noisy-shutdown symptoms.Changes
Each affected site now rethrows
CancellationExceptionbefore the broad catch:DefaultConnection.prepareHeadless/prepareExpert—runCatching {}converted to explicittry/catchDefaultConnection.startListening— inner (emit) and outer (incomingloop) catchesDefaultTab.findElementsByText—resolveNodeand element-build catchesDefaultBrowser.testConnectionDefaultElement.mouseMoveWithTrajectoryviewport queryNon-cancellation behavior is preserved (other exceptions remain swallowed/logged exactly as before). Sites that already rethrow (
querySelector/querySelectorAll,mouseClickcleanup) were left unchanged to keep the change scoped to the root cause.Verification (red → green)
Added
CancellationPropagationTest, which drives the realfindElementsByTextvia aDefaultTabsubclass with a stubbedcallCommand(no browser):DOM.resolveNodethrowsCancellationException, which is swallowed;findElementsByTextreturns normally →assertFailsWith<CancellationException>fails.Full
:core:jvmTest(real headless Chrome) and:opentelemetry:jvmTestpass with no regressions.Test plan
./gradlew :core:jvmTest --tests "*.CancellationPropagationTest"— red on unfixed code, green after fix./gradlew :core:jvmTest(real Chrome) — all pass./gradlew :opentelemetry:jvmTest— all pass