diff --git a/services/web/frontend/js/features/ide-react/hooks/use-connection-outage-tracker.ts b/services/web/frontend/js/features/ide-react/hooks/use-connection-outage-tracker.ts index fb34355848..a61a289457 100644 --- a/services/web/frontend/js/features/ide-react/hooks/use-connection-outage-tracker.ts +++ b/services/web/frontend/js/features/ide-react/hooks/use-connection-outage-tracker.ts @@ -14,7 +14,6 @@ import { useProjectContext } from '@/shared/context/project-context' import type { IdeEvents } from '@/features/ide-react/create-ide-event-emitter' import type { OpenDocuments } from '@/features/ide-react/editor/open-documents' -const OUTAGE_THRESHOLD_MS = 30_000 const EDIT_SNAPSHOT_INTERVAL_MS = 2_000 const MAX_RELOAD_RESTORE_MS = 24 * 60 * 60 * 1000 @@ -70,43 +69,45 @@ function getOldestUnsavedOpAt(openDocs: OpenDocuments): number | null { // spannedReload is independent of the resolution: a reload during the outage // discards the in-memory op queue, so the work only survived if the offline // backup held it, however the outage went on to end. +// +// An outage that ended with no unsaved work is dropped, since we can't +// distinguish between someone reading while offline and an idle tab. function emitOutageEnded( record: ConnectionOutageRecord, { resolution, spannedReload, }: { resolution: OutageResolution; spannedReload: boolean } -) { +): void { const props = readOutageProps(record) - if ( - props.outageDurationMs >= OUTAGE_THRESHOLD_MS || - props.unsavedDurationMs >= OUTAGE_THRESHOLD_MS - ) { - sendMB('connection-restored', { - ...props, - resolution, - spannedReload, - offlineBackupEnabled, - }) + if (props.pendingChars === 0 && props.inflightChars === 0) { + return } - return props -} - -// Reported whatever the duration: sub-threshold teardowns are doc errors -// unrelated to connectivity, and separating those out needs them in the data. -function emitTeardown(record: ConnectionOutageRecord, spannedReload: boolean) { - const { teardownAt } = record - if (teardownAt === undefined) { - return null - } - const props = readOutageProps(record, teardownAt) sendMB('connection-restored', { ...props, + resolution, + spannedReload, + offlineBackupEnabled, + }) +} + +// Sent whether or not any work was unsaved: a teardown is terminal and rare, +// and one with no unsaved work is a doc error unrelated to connectivity, which +// needs to stay separable in the data. +function emitTeardown( + record: ConnectionOutageRecord, + spannedReload: boolean +): void { + const { teardownAt } = record + if (teardownAt === undefined) { + return + } + sendMB('connection-restored', { + ...readOutageProps(record, teardownAt), resolution: 'out-of-sync' satisfies OutageResolution, spannedReload, offlineBackupEnabled, }) - return props } export default function useConnectionOutageTracker(): void { @@ -120,10 +121,6 @@ export default function useConnectionOutageTracker(): void { const prevWebsocketDisconnectedRef = useRef(websocketDisconnected) const prevStalledRef = useRef(stalled) - // The outage record is removed as soon as the connection is back, but the - // offline edits are only acknowledged (or rejected) some time after that, so - // keep the last known outage around to attach to the sync outcome. - const lastOutageRef = useRef(null) // The socket starts out CLOSED, so every mount looks disconnected for a beat. // Nothing before the first connection is an outage. const hasConnectedRef = useRef(isConnected) @@ -153,14 +150,14 @@ export default function useConnectionOutageTracker(): void { // flushed it inline already, but had no way to tell whether the beacon // landed, so report it again here and let detectedAt collapse the pair. if (record.teardownAt !== undefined) { - lastOutageRef.current = emitTeardown(record, true) + emitTeardown(record, true) ConnectionOutageTracker.remove(projectId) return } // Already recovered by mount time, emit and clean up if (!stalled) { - lastOutageRef.current = emitOutageEnded(record, { + emitOutageEnded(record, { resolution: 'reconnected', spannedReload: true, }) @@ -194,7 +191,7 @@ export default function useConnectionOutageTracker(): void { return } teardownFlushedRef.current = true - lastOutageRef.current = emitTeardown(record, spannedReloadRef.current) + emitTeardown(record, spannedReloadRef.current) }, [outOfSync, projectId]) useEffect(() => { @@ -258,7 +255,7 @@ export default function useConnectionOutageTracker(): void { ) { const record = ConnectionOutageTracker.read(projectId) if (record && record.teardownAt === undefined) { - lastOutageRef.current = emitOutageEnded(record, { + emitOutageEnded(record, { resolution: 'reconnected', spannedReload: spannedReloadRef.current, }) @@ -275,7 +272,7 @@ export default function useConnectionOutageTracker(): void { if (prevStalledRef.current === true && stalled === false) { const record = ConnectionOutageTracker.read(projectId) if (record && record.teardownAt === undefined) { - lastOutageRef.current = emitOutageEnded(record, { + emitOutageEnded(record, { resolution: 'saved', spannedReload: spannedReloadRef.current, }) diff --git a/services/web/test/frontend/features/ide-react/hooks/use-connection-outage-tracker.spec.tsx b/services/web/test/frontend/features/ide-react/hooks/use-connection-outage-tracker.spec.tsx index fab6f8ce67..3e7547a4be 100644 --- a/services/web/test/frontend/features/ide-react/hooks/use-connection-outage-tracker.spec.tsx +++ b/services/web/test/frontend/features/ide-react/hooks/use-connection-outage-tracker.spec.tsx @@ -187,7 +187,7 @@ describe('useConnectionOutageTracker', function () { ConnectionOutageTracker.clearAll() }) - it('emits connection-restored once on websocket reconnect after >= 30s offline', function () { + it('emits connection-restored once on websocket reconnect', function () { const controller = {} as Controller mount(controller) @@ -220,11 +220,11 @@ describe('useConnectionOutageTracker', function () { }) }) - it('emits an idle outage with unsavedDurationMs: 0 so no-edit blips still count', function () { - // Socket drops with no ops pending and none arrive during the outage. - // The record's oldestUnsavedOpAt stays null, and unsavedDurationMs must be 0 - // rather than falling back to detectedAt (which would misreport 31s of work - // at risk when there was none). + it('drops an outage that ended with no unsaved work', function () { + // Socket drops with no ops pending and none arrive during the outage, so no + // work was ever at risk. Such an outage is indistinguishable from an idle + // tab or a sleeping machine and is not reported, but the record is still + // cleaned up. const controller = {} as Controller mount(controller) @@ -234,16 +234,11 @@ describe('useConnectionOutageTracker', function () { cy.tick(31_000) cy.then(() => controller.set({ isConnected: true })) cy.tick(1) // flush FIRE passive effect + cy.findByTestId('stalled').should('have.text', 'false') - cy.get('@sendBeacon').should('have.been.calledOnce') - beaconSegmentation(BEACON_URL).should(segmentation => { - expect(segmentation).to.include({ - resolution: 'reconnected', - unsavedDurationMs: 0, - pendingChars: 0, - inflightChars: 0, - }) - expect(segmentation.outageDurationMs).to.be.at.least(31_000) + cy.get('@sendBeacon').should('not.have.been.called') + cy.then(() => { + expect(ConnectionOutageTracker.read(PROJECT_ID)).to.equal(null) }) }) @@ -438,11 +433,13 @@ describe('useConnectionOutageTracker', function () { mount(controller) cy.tick(1) // flush RELOAD-survival mount effect - // Sent despite being far below the 30s threshold cy.get('@sendBeacon').should('have.been.calledOnce') beaconSegmentation(BEACON_URL).should(segmentation => { + // No unsaved work, but teardowns bypass the no-unsaved-work filter expect(segmentation).to.include({ resolution: 'out-of-sync', + pendingChars: 0, + inflightChars: 0, }) expect(segmentation.unsavedDurationMs).to.equal(0) }) @@ -463,8 +460,16 @@ describe('useConnectionOutageTracker', function () { const controller = {} as Controller mount(controller) - // First outage: drop while saving is already stalled - cy.then(() => controller.set({ isConnected: false, isSavingStalled: true })) + // First outage: drop while saving is already stalled. Unsaved work + // throughout, so neither outage is filtered out. + cy.then(() => + controller.set({ + isConnected: false, + isSavingStalled: true, + pendingChars: 4, + oldestOpAgeMs: 0, + }) + ) cy.tick(1) // flush START + SNAPSHOT passive effects cy.findByTestId('stalled').should('have.text', 'true') cy.tick(31_000) @@ -493,7 +498,11 @@ describe('useConnectionOutageTracker', function () { mount(controller) cy.then(() => - controller.set({ isSavingStalled: true, oldestOpAgeMs: 20_000 }) + controller.set({ + isSavingStalled: true, + pendingChars: 3, + oldestOpAgeMs: 20_000, + }) ) cy.tick(1) // flush START + SNAPSHOT passive effects cy.findByTestId('stalled').should('have.text', 'true') @@ -509,26 +518,38 @@ describe('useConnectionOutageTracker', function () { }) }) - it('does not emit when offline for less than 30s', function () { + it('emits a short outage, so brief blips are reported too', function () { const controller = {} as Controller mount(controller) - cy.then(() => controller.set({ isConnected: false })) - cy.tick(1) // flush START passive effect + cy.then(() => + controller.set({ isConnected: false, pendingChars: 2, oldestOpAgeMs: 0 }) + ) + cy.tick(1) // flush START + SNAPSHOT passive effects cy.findByTestId('stalled').should('have.text', 'true') - cy.tick(20_000) + cy.tick(2_000) cy.then(() => controller.set({ isConnected: true })) cy.tick(1) // flush FIRE passive effect cy.findByTestId('stalled').should('have.text', 'false') - cy.get('@sendBeacon').should('not.have.been.called') + cy.get('@sendBeacon').should('have.been.calledOnce') + beaconSegmentation(BEACON_URL).should(segmentation => { + expect(segmentation).to.include({ resolution: 'reconnected' }) + expect(segmentation.outageDurationMs).to.be.lessThan(30_000) + }) }) it('emits a saved resolution for a saving-stalled-only episode (no websocket drop)', function () { const controller = {} as Controller mount(controller) - cy.then(() => controller.set({ isSavingStalled: true })) + cy.then(() => + controller.set({ + isSavingStalled: true, + pendingChars: 3, + oldestOpAgeMs: 0, + }) + ) cy.tick(1) // flush START + SNAPSHOT passive effects cy.findByTestId('stalled').should('have.text', 'true') cy.tick(31_000) @@ -546,7 +567,14 @@ describe('useConnectionOutageTracker', function () { const controller = {} as Controller mount(controller) - cy.then(() => controller.set({ isConnected: false, isSavingStalled: true })) + cy.then(() => + controller.set({ + isConnected: false, + isSavingStalled: true, + pendingChars: 3, + oldestOpAgeMs: 0, + }) + ) cy.tick(1) // flush START + SNAPSHOT passive effects cy.findByTestId('stalled').should('have.text', 'true') cy.tick(31_000) @@ -788,7 +816,7 @@ describe('useConnectionOutageTracker', function () { cy.tick(1) // flush FIRE passive effect cy.findByTestId('stalled').should('have.text', 'false') - // The record is gone by now, so the props come from the stash. + // The outage is over and its record removed, so the sync event stands alone. cy.then(() => { expect(ConnectionOutageTracker.read(PROJECT_ID)).to.equal(null) controller.eventEmitter.emit('ide:offlineChangesSynced', {