Skip to content

Fix unhandled_throwing_task false positives for throws inside nested closures - #6954

Open
Marve10s wants to merge 1 commit into
realm:mainfrom
Marve10s:fix/unhandled-throwing-task-nested-closures
Open

Marve10s wants to merge 1 commit into
realm:mainfrom
Marve10s:fix/unhandled-throwing-task-nested-closures

Conversation

@Marve10s

Copy link
Copy Markdown

The unhandled_throwing_task rule flags Tasks whose body only mentions try inside a nested closure, even when that error can never escape into the Task:

Task {
  await toOptional(closure: { try await exec() })
}

Task {
  let result = try? logPerformance {
    try throwingFunc()
  }
}

Both were reported as unhandled throwing Tasks. The root cause is the rule's ThrowsVisitor: it walked the whole Task call and descended into every nested closure, counting a closure's own try and throw statements as the Task's. A Result initializer special case papered over one instance of this.

This change starts the traversal at the Task operation's statements and skips nested closure bodies. That stays sound without type information because any error that does escape a nested closure into the Task body has to pass through a try or throw in the body itself: Swift requires try at rethrows calls given a throwing closure (so Task { try [1].map { try throwingFunc() } } is still reported), at immediately-invoked throwing closures, and around throwing capture-list initializers, which the visitor still walks. Errors under try? and try! count as handled for the whole expression, matching the existing examples. With nested closures skipped generally, the Result special case is no longer needed and is removed. Parenthesized and as-cast operation closures passed to Task(operation:) are unwrapped so they keep being checked.

The new rule examples cover the two reported false positives plus the cases that must keep triggering: a throwing map closure, a nested Task (only the inner one is reported), and an unhandled throwing capture initializer.

Refs #4987. This covers the nested-closure false positives from that issue; the separate case of a throwing Task passed as a function argument is left for a dedicated rule, as discussed there.

@SwiftLintBot

Copy link
Copy Markdown
3 Warnings
⚠️ This PR may need tests.
⚠️ This PR changed the exit value from pid 46949 exit 2 to pid 46725 exit 0 when running in NetNewsWire.
⚠️ This PR changed the exit value from pid 46971 exit 2 to pid 46747 exit 0 when running in PocketCasts.
21 Messages
📖 Building this branch resulted in a binary size of 28772.04 KiB vs 28755.54 KiB when built on main (0% larger).
📖 Linting Aerial with this PR took 0.1 s vs 0.1 s on main (0% slower).
📖 Linting Alamofire with this PR took 0.11 s vs 0.13 s on main (15% faster).
📖 Linting Brave with this PR took 0.37 s vs 0.36 s on main (2% slower).
📖 Linting Brigade with this PR took 18.69 s vs 18.64 s on main (0% slower).
📖 Linting DuckDuckGo with this PR took 2.08 s vs 2.1 s on main (0% faster).
📖 Linting Firefox with this PR took 0.73 s vs 0.72 s on main (1% slower).
📖 Linting Kickstarter with this PR took 0.44 s vs 0.44 s on main (0% slower).
📖 Linting Moya with this PR took 0.09 s vs 0.09 s on main (0% slower).
📖 Linting NetNewsWire with this PR took 0.18 s vs 0.18 s on main (0% slower).
📖 Linting Nimble with this PR took 0.09 s vs 0.09 s on main (0% slower).
📖 Linting PocketCasts with this PR took 0.5 s vs 0.48 s on main (4% slower).
📖 Linting Quick with this PR took 0.09 s vs 0.09 s on main (0% slower).
📖 Linting Realm with this PR took 0.19 s vs 0.19 s on main (0% slower).
📖 Linting Sourcery with this PR took 0.15 s vs 0.15 s on main (0% slower).
📖 Linting Swift with this PR took 0.21 s vs 0.21 s on main (0% slower).
📖 Linting SwiftLintPerformanceTests with this PR took 3.75 s vs 3.78 s on main (0% faster).
📖 Linting VLC with this PR took 0.18 s vs 0.18 s on main (0% slower).
📖 Linting Wire with this PR took 0.75 s vs 0.74 s on main (1% slower).
📖 Linting WordPress with this PR took 0.57 s vs 0.55 s on main (3% slower).
📖 This PR fixed a violation in DuckDuckGo: /SharedPackages/VPN/Sources/VPN/Monitoring/TunnelMonitors.swift:168:9: Error: Errors thrown inside this task are not handled, which may be unexpected. Handle errors inside the task, or use try await to access the Tasks value and handle errors. See this forum thread for more details: https://forums.swift.org/t/task-initializer-with-throwing-closure-swallows-error/56066 (unhandled_throwing_task)

Generated by 🚫 Danger

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants