ColtenOuO opened a new pull request, #70141:
URL: https://github.com/apache/airflow/pull/70141
## Summary
`heartbeater.Run` in the Go SDK worker checks whether a heartbeat error
indicates the server no longer wants the task running (404/409), so it can
cancel the task instead of retrying forever:
```go
if resp != nil && resp.StatusCode() == 404 || resp.StatusCode() == 409 {
```
`&&` binds tighter than `||` in Go, so this actually parses as `(resp != nil
&& resp.StatusCode() == 404) || resp.StatusCode() == 409` -- the second
disjunct calls `resp.StatusCode()` unconditionally, bypassing the nil check the
`resp != nil &&` clearly intended to guard.
`resp` here is `*api.GeneralHTTPError.Response` (`*resty.Response`), which
can be nil for transport-level failures that never got as far as an actual HTTP
response (e.g. connection reset, TLS failure). When that happens,
`resp.StatusCode()` panics on a nil pointer -- and this happens inside the
heartbeater's own goroutine (`go heartbeater.Run(heartbeatCtx)` in
`ExecuteTaskWorkload`), which has no `recover()` of its own. Go's `recover()`
only catches panics in the same goroutine's call stack, so this crashes the
entire worker process, not just the one task.
Confirmed by temporarily reverting the fix and running the new test: it
panics with `SIGSEGV` at exactly this line, unrecovered, exactly as described
above.
## Changes
- Add parentheses so the nil guard covers both status checks:
`resp != nil && (resp.StatusCode() == 404 || resp.StatusCode() == 409)`.
- Add `TestTaskHeartbeatNilResponseDoesNotPanic`, which returns a
`GeneralHTTPError{Response: nil}` from the mocked heartbeat call and
asserts the task completes normally instead of crashing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
