Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-scene-drops.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@viamrobotics/motion-tools': patch
---

fix: stream auto-reconnect for draw service and preserve scene entities during disconnect
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@
"@typescript-eslint/parser": "8.56.1",
"@viamrobotics/prime-core": "0.1.5",
"@viamrobotics/sdk": "0.69.0",
"@viamrobotics/svelte-sdk": "1.2.2",
"@viamrobotics/svelte-sdk": "1.2.3",
"@viamrobotics/tweakpane-config": "0.1.1",
"@vitest/browser-playwright": "4.1.9",
"@vitest/coverage-v8": "4.1.9",
Expand Down
39 changes: 27 additions & 12 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

154 changes: 154 additions & 0 deletions src/lib/__tests__/retry-stream.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
import { describe, expect, it, vi } from 'vitest'

import { retryStream } from '../retry-stream'

describe('retryStream', () => {
it('calls run and resolves when run succeeds', async () => {
const run = vi.fn().mockResolvedValue(undefined)
const controller = new AbortController()

// run resolves once, retryStream will call it again — abort after first call
run.mockImplementation(async () => {
controller.abort()
})

await retryStream(run, controller.signal)

expect(run).toHaveBeenCalledTimes(1)
})

it('retries when run throws', async () => {
vi.useFakeTimers()

const controller = new AbortController()
let callCount = 0

const run = vi.fn().mockImplementation(async () => {
callCount++
if (callCount < 3) {
throw new Error('stream error')
}
controller.abort()
})

const promise = retryStream(run, controller.signal)
// Advance through the backoff delays
await vi.advanceTimersByTimeAsync(1_000)
await vi.advanceTimersByTimeAsync(2_000)

await promise

expect(run).toHaveBeenCalledTimes(3)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test: delay cap at MAX_DELAY_MS (30 s) is untested.

The exponential doubling is exercised up to 2 s but never hits the 30 s ceiling. Worth one parameterized test that runs enough failures to saturate the cap and verifies onRetry stops receiving larger values.

Also missing: a test for what happens when signal is already aborted before retryStream is called (the while (!signal.aborted) check on the first iteration). The current suite only aborts during an in-flight operation or backoff.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude (@claude) add these tests

@claude claude Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude finished Devin T. Currie (@DTCurrie)'s task in 4m 12s —— View job


  • Read CLAUDE.md and test file context
  • Read retry-stream implementation
  • Add test for MAX_DELAY_MS cap (30s ceiling)
  • Add test for pre-aborted signal
  • Commit and push

Both missing tests have been added in commit 4e925fb:

caps delay at MAX_DELAY_MS (30s) and does not double beyond it — runs 6 consecutive failures to drive the backoff through 1s → 2s → 4s → 8s → 16s → 30s (capped), then asserts the 6th onRetry call received 30_000 rather than 32_000.

does not call run when signal is already aborted before retryStream is called — pre-aborts the controller before passing the signal, asserts run is never invoked (tests the while (!signal.aborted) guard on first entry).
· Branch


vi.useRealTimers()
})

it('stops retrying when signal is aborted', async () => {
vi.useFakeTimers()

const controller = new AbortController()
const run = vi.fn().mockRejectedValue(new Error('stream error'))
const onRetry = vi.fn()

const promise = retryStream(run, controller.signal, onRetry)

// First call fails immediately, then waits for backoff
await vi.advanceTimersByTimeAsync(0)
expect(run).toHaveBeenCalledTimes(1)

// Abort during backoff wait
controller.abort()
await vi.advanceTimersByTimeAsync(1_000)

await promise

// Should have called onRetry once, but not retried run
expect(onRetry).toHaveBeenCalledTimes(1)
Comment thread
DTCurrie marked this conversation as resolved.
expect(run).toHaveBeenCalledTimes(1)

vi.useRealTimers()
})

it('calls onRetry with the current delay', async () => {
vi.useFakeTimers()

const controller = new AbortController()
let callCount = 0

const run = vi.fn().mockImplementation(async () => {
callCount++
if (callCount < 3) {
throw new Error('stream error')
}
controller.abort()
})

const onRetry = vi.fn()
const promise = retryStream(run, controller.signal, onRetry)

await vi.advanceTimersByTimeAsync(1_000)
await vi.advanceTimersByTimeAsync(2_000)

await promise

expect(onRetry).toHaveBeenCalledTimes(2)
expect(onRetry).toHaveBeenNthCalledWith(1, 1_000)
expect(onRetry).toHaveBeenNthCalledWith(2, 2_000)

vi.useRealTimers()
})

it('does not call onRetry and restarts immediately on clean stream end', async () => {
const controller = new AbortController()
let callCount = 0

const run = vi.fn().mockImplementation(async () => {
callCount++
if (callCount === 1) return // clean end — server closed the stream
controller.abort()
})

const onRetry = vi.fn()
await retryStream(run, controller.signal, onRetry)

expect(run).toHaveBeenCalledTimes(2)
expect(onRetry).not.toHaveBeenCalled()
})

it('resets delay after a successful run', async () => {
vi.useFakeTimers()

const controller = new AbortController()
let callCount = 0

const run = vi.fn().mockImplementation(async () => {
callCount++
// First call: fail
if (callCount === 1) throw new Error('fail')
// Second call: succeed (stream ended cleanly)
if (callCount === 2) return
// Third call: fail
if (callCount === 3) throw new Error('fail')
// Fourth call: abort
controller.abort()
})

const onRetry = vi.fn()
const promise = retryStream(run, controller.signal, onRetry)

// First failure + 1s backoff
await vi.advanceTimersByTimeAsync(1_000)
// Second call succeeds, delay resets. Third call fails, should use 1s again
await vi.advanceTimersByTimeAsync(1_000)
// Fourth call - abort
await vi.advanceTimersByTimeAsync(2_000)

await promise

// Both retries should have used 1000ms (reset after success)
expect(onRetry).toHaveBeenNthCalledWith(1, 1_000)
expect(onRetry).toHaveBeenNthCalledWith(2, 1_000)

vi.useRealTimers()
})
})
24 changes: 16 additions & 8 deletions src/lib/hooks/useGeometries.svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,19 +222,27 @@ export const provideGeometries = (partID: () => string) => {
})
}

// Clean up owners whose queries disappeared entirely
// Clean up owners whose queries disappeared entirely.
// Guard: if ALL queries are gone (activeQueryKeys empty), the machine is likely
// temporarily disconnected — preserve entities so they reappear on reconnect.
// Only destroy when the partID changed (old-partID entities) or other queries
// are still active (connected machine, resource legitimately removed).
const anyQueriesActive = activeQueryKeys.size > 0
for (const [queryKey, keys] of queryEntityKeys) {
if (!activeQueryKeys.has(queryKey)) {
for (const key of keys) {
const entity = entities.get(key)
if (entity && world.has(entity)) {
entity.destroy()
const queryPartID = queryKey.split(':')[0]!
if (queryPartID !== currentPartID || anyQueriesActive) {
Comment thread
claude[bot] marked this conversation as resolved.
for (const key of keys) {
const entity = entities.get(key)
if (entity && world.has(entity)) {
entity.destroy()
}

entities.delete(key)
}

entities.delete(key)
queryEntityKeys.delete(queryKey)
}

queryEntityKeys.delete(queryKey)
}
}
})
Expand Down
16 changes: 12 additions & 4 deletions src/lib/hooks/usePointclouds.svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,21 @@ export const providePointclouds = (partID: () => string) => {
})
}

// clean up queries that disappeared entirely
// clean up queries that disappeared entirely.
// Guard: if ALL queries are gone (activeQueryKeys empty), the machine is likely
// temporarily disconnected — preserve entities so they reappear on reconnect.
// Only destroy when the partID changed (old-partID entities) or other queries
// are still active (connected machine, camera legitimately removed).
const anyQueriesActive = activeQueryKeys.size > 0
for (const [queryKey, entity] of entities) {
if (!activeQueryKeys.has(queryKey)) {
if (world.has(entity)) {
entity.destroy()
const queryPartID = queryKey.split(':')[0]!
Comment thread
claude[bot] marked this conversation as resolved.
if (queryPartID !== currentPartID || anyQueriesActive) {
if (world.has(entity)) {
entity.destroy()
}
entities.delete(queryKey)
}
entities.delete(queryKey)
}
}
})
Expand Down
Loading
Loading