Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion apps/desktop/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@zennotes/desktop",
"productName": "ZenNotes",
"version": "2.44.0",
"version": "2.45.0",
"description": "ZenNotes desktop shell",
"private": true,
"main": "./out/main/index.js",
Expand Down
25 changes: 16 additions & 9 deletions apps/desktop/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1835,6 +1835,16 @@ function stopRemoteWatch(): void {
}
}

/** Tell every remote window to re-pull the vault: its list is behind the server. */
function sendRemoteResync(): void {
windowVaults.sendRemoteVaultChange({
kind: "change",
path: "",
folder: "inbox",
scope: "resync",
});
}

function startRemoteWatch(
client: RemoteServerClient,
capabilities: ServerCapabilities,
Expand All @@ -1854,14 +1864,7 @@ function startRemoteWatch(
windowVaults.sendRemoteVaultChange(ev);
},
{
onReconnect: () => {
windowVaults.sendRemoteVaultChange({
kind: "change",
path: "",
folder: "inbox",
scope: "resync",
});
},
onReconnect: () => sendRemoteResync(),
},
);
}
Expand Down Expand Up @@ -2017,7 +2020,11 @@ async function setRemoteWorkspace(
vaultPath?: string | null;
} = {},
): Promise<{ vault: VaultInfo | null; capabilities: ServerCapabilities }> {
const client = new RemoteServerClient({ baseUrl, authToken });
const client = new RemoteServerClient({
baseUrl,
authToken,
onStalePath: () => sendRemoteResync(),
});
let capabilities = await client.getCapabilities();
remoteWorkspaceBootError = null;
let vault = await client.getCurrentVault();
Expand Down
92 changes: 89 additions & 3 deletions apps/desktop/src/main/remote/server-client.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import http from 'node:http'
import type { AddressInfo } from 'node:net'
import { WebSocketServer } from 'ws'
import { describe, expect, it } from 'vitest'
import { describe, expect, it, vi } from 'vitest'
import {
connectionErrorMessage,
RemoteConnectionError,
RemoteRequestError,
RemoteServerClient
RemoteServerClient,
connectionErrorMessage
} from './server-client'

describe('connectionErrorMessage (#481)', () => {
Expand Down Expand Up @@ -214,6 +214,41 @@ describe('watchVaultChanges reconnect', () => {
}
}, 15_000)

it('a proxy that refuses the upgrade falls back to polling, and stop ends the polling (#734)', async () => {
// A reverse proxy without WebSocket support answers the handshake with a
// plain HTTP error every time. The feed is not briefly down, it is
// unavailable, and the old client left the vault frozen at connect time.
const server = http.createServer((_req, res) => {
res.writeHead(404)
res.end('not found')
})
server.on('upgrade', (_req, socket) => {
socket.end('HTTP/1.1 404 Not Found\r\nContent-Length: 9\r\n\r\nnot found')
})
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
const { port } = server.address() as AddressInfo
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
const client = new RemoteServerClient({ baseUrl: `http://127.0.0.1:${port}` })

let resyncs = 0
const stop = client.watchVaultChanges(() => {}, {
onReconnect: () => (resyncs += 1),
pollWhileDownMs: 100
})
try {
await waitFor(() => resyncs >= 3, 5_000, 'polling resyncs')
expect(warn).toHaveBeenCalledTimes(1)
expect(String(warn.mock.calls[0][0])).toContain('/api/watch')
} finally {
stop()
warn.mockRestore()
}
const afterStop = resyncs
await new Promise((resolve) => setTimeout(resolve, 350))
expect(resyncs).toBe(afterStop)
await new Promise((resolve) => server.close(resolve))
}, 10_000)

it('an unreachable server neither throws nor crashes, and stop cancels the retry loop', async () => {
// Port 1 is never listening. The connection error must stay inside the
// client (an unhandled ws 'error' event would crash the process, which
Expand All @@ -225,3 +260,54 @@ describe('watchVaultChanges reconnect', () => {
await new Promise((resolve) => setTimeout(resolve, 100))
}, 10_000)
})


describe('a 404 for a path this app asked to change (#734)', () => {
async function serverAnswering(status: number, body: string): Promise<{ port: number; close: () => Promise<void>; requests: string[] }> {
const requests: string[] = []
const server = http.createServer((req, res) => {
requests.push(`${req.method} ${req.url}`)
res.writeHead(status)
res.end(body)
})
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
const { port } = server.address() as AddressInfo
return { port, requests, close: () => new Promise((resolve) => server.close(() => resolve())) }
}

it('names the path, keeps the 404 status, and asks the host to re-pull the list', async () => {
const { port, close } = await serverAnswering(404, 'not found')
const stale: string[] = []
const client = new RemoteServerClient({
baseUrl: `http://127.0.0.1:${port}`,
onStalePath: (path) => stale.push(path)
})
try {
const error = await client.moveToTrash('inbox/Renamed elsewhere.md').catch((e: unknown) => e)
expect(error).toBeInstanceOf(RemoteRequestError)
expect((error as RemoteRequestError).status).toBe(404)
expect((error as Error).message).toContain('nothing at inbox/Renamed elsewhere.md any more')
expect((error as Error).message).toContain('refreshed')
expect(stale).toEqual(['inbox/Renamed elsewhere.md'])
} finally {
await close()
}
})

it('leaves a 404 on a read alone: absent is a valid answer there (#556)', async () => {
const { port, close } = await serverAnswering(404, 'not found')
const stale: string[] = []
const client = new RemoteServerClient({
baseUrl: `http://127.0.0.1:${port}`,
onStalePath: (path) => stale.push(path)
})
try {
const error = await client.readNote('inbox/Absent.md').catch((e: unknown) => e)
expect((error as RemoteRequestError).status).toBe(404)
expect((error as Error).message).toContain('404')
expect(stale).toEqual([])
} finally {
await close()
}
})
})
61 changes: 60 additions & 1 deletion apps/desktop/src/main/remote/server-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ import {
export interface RemoteServerClientOptions {
baseUrl: string
authToken?: string | null
/**
* Called with the vault-relative path when the server answers 404 to a
* request about it. The list this app shows is behind the server, so the
* host re-pulls it (see `stalePathMessage`).
*/
onStalePath?: (path: string) => void
}

type JsonRequestInit = Omit<RequestInit, 'body'> & { body?: unknown }
Expand All @@ -48,6 +54,7 @@ import type {
WriteWorkflowInput
} from '@zennotes/bridge-contract/workflows'
import { prepareWorkflowRun } from '@shared/workflows/prepare-run'
import { REMOTE_CHANGE_POLL_MS, stalePathMessage } from '@shared/remote-workspace-messages'

export class RemoteConnectionError extends Error {}

Expand All @@ -65,10 +72,12 @@ export class RemoteRequestError extends Error {
export class RemoteServerClient {
readonly baseUrl: string
readonly authToken: string | null
private readonly onStalePath: ((path: string) => void) | null

constructor(options: RemoteServerClientOptions) {
this.baseUrl = normalizeBaseUrl(options.baseUrl)
this.authToken = options.authToken?.trim() || null
this.onStalePath = options.onStalePath ?? null
}

async getCapabilities(): Promise<ServerCapabilities> {
Expand Down Expand Up @@ -452,7 +461,7 @@ export class RemoteServerClient {

watchVaultChanges(
onEvent: (event: VaultChangeEvent) => void,
options: { onReconnect?: () => void; stableAfterMs?: number } = {}
options: { onReconnect?: () => void; stableAfterMs?: number; pollWhileDownMs?: number } = {}
): () => void {
const url = new URL('/api/watch', `${this.baseUrl}/`)
const headers: Record<string, string> = {}
Expand All @@ -473,6 +482,35 @@ export class RemoteServerClient {
let failedAttempts = 0
// How long a socket must stay up before it counts as a real session.
const stableAfterMs = options.stableAfterMs ?? 15_000
// Some hosts never let the socket through at all: a reverse proxy that
// does not forward the Upgrade handshake answers every attempt with a
// plain HTTP error, so the feed is not "briefly down", it is unavailable.
// Left alone, this app then shows a vault frozen at connect time, and a
// note another device renamed or trashed still lists under its old path
// until an operation on it comes back 404 (#734). While the socket is
// down, re-pull the vault on a timer instead; each tick is a gap the
// caller closes the same way it closes a reconnect.
const pollWhileDownMs = options.pollWhileDownMs ?? REMOTE_CHANGE_POLL_MS
let pollTimer: NodeJS.Timeout | null = null
let warnedAboutPolling = false
const startPolling = (): void => {
if (pollTimer || stopped) return
if (!warnedAboutPolling) {
warnedAboutPolling = true
console.warn(
`[remote] ${this.baseUrl}: the change feed at /api/watch is not staying connected (a proxy without WebSocket support?); refreshing every ${Math.round(pollWhileDownMs / 1000)}s instead`
)
}
pollTimer = setInterval(() => {
if (!stopped) options.onReconnect?.()
}, pollWhileDownMs)
}
const stopPolling = (): void => {
if (pollTimer) {
clearInterval(pollTimer)
pollTimer = null
}
}

const connect = (): void => {
if (stopped) return
Expand All @@ -484,6 +522,7 @@ export class RemoteServerClient {
// caller re-pulls everything instead of trusting the resumed feed.
// Only the very first attempt connecting cleanly has no gap.
const hadGap = failedAttempts > 0
stopPolling()
// The failure counter resets only after the socket has stayed up for
// a while, not on the handshake: a peer that accepts the upgrade and
// immediately drops it (a misconfigured proxy, a crash-looping
Expand Down Expand Up @@ -527,6 +566,7 @@ export class RemoteServerClient {
ws = null
const delay = Math.min(30_000, 1_000 * 2 ** failedAttempts)
failedAttempts += 1
startPolling()
reconnectTimer = setTimeout(connect, delay)
})
}
Expand All @@ -535,6 +575,7 @@ export class RemoteServerClient {

return () => {
stopped = true
stopPolling()
if (reconnectTimer) {
clearTimeout(reconnectTimer)
reconnectTimer = null
Expand Down Expand Up @@ -573,6 +614,17 @@ export class RemoteServerClient {
}
if (!response.ok) {
const text = await response.text().catch(() => '')
// A 404 for a path this app asked to change means the list is behind
// the server, not that the server is broken: another device moved,
// renamed, or trashed the note and the change never arrived here
// (#734). Say which path is gone and have the host re-pull the list.
// Reads keep the plain answer: a 404 on `?path=` is how remote
// databases learn a file is absent (#556), and that is not staleness.
const stalePath = response.status === 404 ? requestedPath(init?.body) : null
if (stalePath !== null) {
this.onStalePath?.(stalePath)
throw new RemoteRequestError(stalePathMessage(stalePath), response.status)
}
throw new RemoteRequestError(
requestErrorMessage(this.baseUrl, path, response, text),
response.status
Expand All @@ -582,3 +634,10 @@ export class RemoteServerClient {
return (await response.json()) as T
}
}

/** The vault-relative path a JSON request body names, when it names one. */
function requestedPath(body: unknown): string | null {
if (!body || typeof body !== 'object') return null
const path = (body as { path?: unknown }).path
return typeof path === 'string' && path.length > 0 ? path : null
}
2 changes: 1 addition & 1 deletion apps/server/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@zennotes/server",
"private": true,
"version": "2.44.0",
"version": "2.45.0",
"scripts": {
"dev": "node ../../tooling/scripts/run-go-server-dev.mjs",
"prepare-web": "node ../../tooling/scripts/prepare-server-web-dist.mjs",
Expand Down
2 changes: 1 addition & 1 deletion apps/web/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@zennotes/web",
"private": true,
"version": "2.44.0",
"version": "2.45.0",
"type": "module",
"description": "ZenNotes web client for self-hosted and hosted deployments",
"homepage": "https://zennotes.org",
Expand Down
Loading
Loading