Skip to content

proxy: propagate request cancellation to pending Fleet and command work - #496

Closed
ZayanKhan-12 wants to merge 1 commit into
teslamotors:mainfrom
ZayanKhan-12:fix-491-proxy-request-cancel
Closed

ZayanKhan-12 wants to merge 1 commit into
teslamotors:mainfrom
ZayanKhan-12:fix-491-proxy-request-cancel

Conversation

@ZayanKhan-12

Copy link
Copy Markdown
Contributor

Description

tesla-http-proxy derived per-request timeouts from context.Background(), so canceling the incoming HTTP request did not stop forwarded Fleet API I/O or vehicle lookup/session work. This PR derives the existing timeout from req.Context() on both forwardRequest and handleVehicleCommand, and stops a dispatcher listener if Connect is canceled before the handler would defer Disconnect.

Fixes #491

Related: #492 covers the REST-forwarding line only and deferred the vehicle-command path because of that dispatcher lifecycle. This PR includes that path.

Type of change

Please select all options that apply to this change:

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

Checklist:

Confirm you have completed the following steps:

  • My code follows the style of this project.
  • I have performed a self-review of my code.
  • I have made corresponding updates to the documentation.
  • I have added/updated unit tests to cover my changes.

Summary

Go's server request context is canceled when the client disconnects. The proxy must preserve that alongside p.Timeout.

What this PR does:

  • forwardRequest and handleVehicleCommand use context.WithTimeout(req.Context(), p.Timeout).
  • Dispatcher.Start creates the terminate channel before listen, stops the listener if ctx is canceled before ready, and always signals ready on exit so Start cannot leak the goroutine.
  • handleVehicleCommand calls Disconnect after a failed Connect.
  • Does not recall a command already delivered to the vehicle.

Test plan

  • go build ./... && go test ./... && go vet ./... && gofmt -l .
  • go test -race ./pkg/proxy ./internal/dispatcher
  • Synchronized TestForwardRequestPropagatesRequestCancellation and TestHandleVehicleCommandPropagatesRequestCancellation (no sleeps)
  • TestStartCanceledStopsListener
  • Connect-error path calls Disconnect

Per-request timeouts were derived from context.Background(), so a client disconnect left forwarded Fleet API I/O and vehicle lookup running. Derive the existing timeout from req.Context(), stop a dispatcher listener if Connect is canceled before Disconnect, and always Disconnect after a failed Connect.
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.

tesla-http-proxy does not propagate incoming request cancellation

3 participants