Skip to content

Commit 9e17e3f

Browse files
committed
fix(desktop): keep retrying the origin past a broken offline page and drop the font copy
1 parent 2974e87 commit 9e17e3f

6 files changed

Lines changed: 105 additions & 39 deletions

File tree

apps/desktop/.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,3 @@ release/
33
build/generated-icon.icon
44
playwright-report/
55
test-results/
6-
static/SeasonSansUprightsVF.woff2

apps/desktop/scripts/build.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,6 @@ rmSync(generatedIcon, { force: true, recursive: true })
3232
cpSync(appIcon, generatedIcon, { recursive: true })
3333
console.log(`• Selecting desktop icon: ${appIcon}`)
3434

35-
// The bundled pages load their font from static/ over the shell's own scheme.
36-
// electron-builder copies it there for packaged builds; this does the same for
37-
// unpackaged runs and the e2e suite, so both serve it from one place.
38-
const brandFont = join('..', 'sim', 'public', 'brand', 'fonts', 'SeasonSansUprightsVF.woff2')
39-
cpSync(brandFont, join('static', 'SeasonSansUprightsVF.woff2'))
40-
console.log('• Copied the Season Sans font for the bundled pages')
41-
4235
function compileNativeHelpSearch(): void {
4336
const outputDirectory = 'dist/native'
4437
rmSync(outputDirectory, { force: true, recursive: true })

apps/desktop/src/main/load-health.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,25 @@ describe('attachLoadHealth', () => {
8585
expect(win.loadURL).toHaveBeenCalledTimes(1)
8686
expect(events.record).toHaveBeenCalledTimes(1)
8787
})
88+
89+
// Stopping the retry instead would strand the window blank until a relaunch.
90+
// The origin keeps being retried on the usual cadence; only the broken
91+
// bundled page is never navigated to again.
92+
it('keeps retrying the origin after the offline page broke, without reloading it', () => {
93+
const { win, events, failLoad } = setup()
94+
95+
failLoad(-105, 'ERR_NAME_NOT_RESOLVED', 'https://sim.example.com/workspace')
96+
failLoad(-6, 'ERR_FILE_NOT_FOUND', 'sim-shell://pages/offline.html?kind=dns')
97+
vi.advanceTimersByTime(5000)
98+
99+
expect(win.loadURL).toHaveBeenCalledTimes(2)
100+
expect(win.loadURL).toHaveBeenLastCalledWith('https://sim.example.com/workspace')
101+
102+
failLoad(-105, 'ERR_NAME_NOT_RESOLVED', 'https://sim.example.com/workspace')
103+
vi.advanceTimersByTime(5000)
104+
105+
expect(events.record).toHaveBeenCalledTimes(2)
106+
expect(win.loadURL).toHaveBeenCalledTimes(3)
107+
expect(win.loadURL).toHaveBeenLastCalledWith('https://sim.example.com/workspace')
108+
})
88109
})

apps/desktop/src/main/load-health.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ export interface LoadHealthHandle {
5656
export function attachLoadHealth(win: BrowserWindow, deps: LoadHealthDeps): LoadHealthHandle {
5757
let intendedUrl: string | null = null
5858
let showingOffline = false
59+
let offlinePageBroken = false
5960
let retryTimer: NodeJS.Timeout | undefined
6061
let watchdogTimer: NodeJS.Timeout | undefined
6162

@@ -108,7 +109,12 @@ export function attachLoadHealth(win: BrowserWindow, deps: LoadHealthDeps): Load
108109
}
109110
showingOffline = true
110111
deps.events.record('load_failure', { kind, detail })
111-
void win.loadURL(deps.offlinePageUrl({ kind, detail }))
112+
// A bundled page that failed once fails for a packaging reason, not a
113+
// transient one, so it is never navigated to again. The origin retry stays
114+
// armed regardless: it is the only way the window recovers on its own.
115+
if (!offlinePageBroken) {
116+
void win.loadURL(deps.offlinePageUrl({ kind, detail }))
117+
}
112118
startAutoRetry()
113119
}
114120

@@ -127,6 +133,7 @@ export function attachLoadHealth(win: BrowserWindow, deps: LoadHealthDeps): Load
127133
// page itself failed there is nothing left to swap to, and showing it
128134
// again would loop.
129135
if (showingOffline && !validatedURL?.startsWith('http')) {
136+
offlinePageBroken = true
130137
logger.error('Bundled offline page failed to load', {
131138
errorCode,
132139
errorDescription,

apps/desktop/src/main/local-pages.test.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ describe('createLocalPageHandler', () => {
7373
})
7474

7575
it('serves allowlisted files with their content type', async () => {
76-
const response = await createLocalPageHandler(root)(
76+
const response = await createLocalPageHandler([root])(
7777
new Request(`${LOCAL_PAGE_ORIGIN}/offline.html?kind=dns`)
7878
)
7979

@@ -84,7 +84,7 @@ describe('createLocalPageHandler', () => {
8484
})
8585

8686
it('refuses everything outside the allowlist, however the path is spelled', async () => {
87-
const handler = createLocalPageHandler(root)
87+
const handler = createLocalPageHandler([root])
8888
for (const path of [
8989
'/secret.txt',
9090
'/../secret.txt',
@@ -98,7 +98,7 @@ describe('createLocalPageHandler', () => {
9898
})
9999

100100
it('refuses a foreign host and non-GET methods', async () => {
101-
const handler = createLocalPageHandler(root)
101+
const handler = createLocalPageHandler([root])
102102

103103
expect((await handler(new Request('sim-shell://evil/offline.html'))).status).toBe(404)
104104
expect(
@@ -107,12 +107,32 @@ describe('createLocalPageHandler', () => {
107107
})
108108

109109
it('answers 404 for an allowlisted file that is missing on disk', async () => {
110-
const response = await createLocalPageHandler(root)(
110+
const response = await createLocalPageHandler([root])(
111111
new Request(`${LOCAL_PAGE_ORIGIN}/server.html`)
112112
)
113113

114114
expect(response.status).toBe(404)
115115
})
116+
117+
// Unpackaged runs read the brand font from the web app's public fonts rather
118+
// than a generated copy in static/, so roots are consulted in order.
119+
it('falls through to a later root for an asset the first one lacks', async () => {
120+
const fonts = mkdtempSync(join(tmpdir(), 'sim-local-pages-fonts-'))
121+
writeFileSync(join(fonts, 'SeasonSansUprightsVF.woff2'), 'woff2-bytes')
122+
try {
123+
const handler = createLocalPageHandler([root, fonts])
124+
125+
const font = await handler(new Request(`${LOCAL_PAGE_ORIGIN}/SeasonSansUprightsVF.woff2`))
126+
expect(font.status).toBe(200)
127+
expect(font.headers.get('content-type')).toBe('font/woff2')
128+
expect(await font.text()).toBe('woff2-bytes')
129+
130+
const page = await handler(new Request(`${LOCAL_PAGE_ORIGIN}/offline.html`))
131+
expect(await page.text()).toBe('<h1>offline</h1>')
132+
} finally {
133+
rmSync(fonts, { recursive: true, force: true })
134+
}
135+
})
116136
})
117137

118138
describe('attachLocalPageProtocol', () => {
@@ -121,11 +141,11 @@ describe('attachLocalPageProtocol', () => {
121141
protocol: { isProtocolHandled: vi.fn(() => false), handle: vi.fn() },
122142
}
123143

124-
attachLocalPageProtocol(ses as unknown as Session, '/tmp/static')
144+
attachLocalPageProtocol(ses as unknown as Session, ['/tmp/static'])
125145
expect(ses.protocol.handle).toHaveBeenCalledWith('sim-shell', expect.any(Function))
126146

127147
ses.protocol.isProtocolHandled.mockReturnValue(true)
128-
attachLocalPageProtocol(ses as unknown as Session, '/tmp/static')
148+
attachLocalPageProtocol(ses as unknown as Session, ['/tmp/static'])
129149
expect(ses.protocol.handle).toHaveBeenCalledTimes(1)
130150
})
131151
})

apps/desktop/src/main/local-pages.ts

Lines changed: 50 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { extname, join } from 'node:path'
33
import { createLogger } from '@sim/logger'
44
import { getErrorMessage } from '@sim/utils/errors'
55
import type { Session } from 'electron'
6-
import { protocol } from 'electron'
6+
import { app, protocol } from 'electron'
77

88
const logger = createLogger('DesktopLocalPages')
99

@@ -94,10 +94,12 @@ function notFound(): Response {
9494
}
9595

9696
/**
97-
* Serves allowlisted files from `rootDir`. Split from the session wiring so
98-
* tests can drive it against a temporary directory.
97+
* Serves allowlisted files from the first of `rootDirs` that has them. Split
98+
* from the session wiring so tests can drive it against temporary directories.
9999
*/
100-
export function createLocalPageHandler(rootDir: string): (request: Request) => Promise<Response> {
100+
export function createLocalPageHandler(
101+
rootDirs: readonly string[]
102+
): (request: Request) => Promise<Response> {
101103
return async (request) => {
102104
if (request.method !== 'GET') {
103105
return new Response(null, { status: 405 })
@@ -115,41 +117,65 @@ export function createLocalPageHandler(rootDir: string): (request: Request) => P
115117
if (!SERVABLE_FILES.has(name)) {
116118
return notFound()
117119
}
120+
const file = await readFirst(rootDirs, name)
121+
if (!file) {
122+
return notFound()
123+
}
124+
// A copy into a plain ArrayBuffer: Response bodies take BufferSource, and
125+
// a Node Buffer's backing store is not typed as one.
126+
const body = new Uint8Array(file.byteLength)
127+
body.set(file)
128+
return new Response(body.buffer, {
129+
status: 200,
130+
headers: {
131+
'Content-Type': CONTENT_TYPES[extname(name)] ?? 'application/octet-stream',
132+
'X-Content-Type-Options': 'nosniff',
133+
},
134+
})
135+
}
136+
}
137+
138+
async function readFirst(rootDirs: readonly string[], name: string): Promise<Buffer | null> {
139+
for (const rootDir of rootDirs) {
118140
try {
119-
const file = await readFile(join(rootDir, name))
120-
// A copy into a plain ArrayBuffer: Response bodies take BufferSource, and
121-
// a Node Buffer's backing store is not typed as one.
122-
const body = new Uint8Array(file.byteLength)
123-
body.set(file)
124-
return new Response(body.buffer, {
125-
status: 200,
126-
headers: {
127-
'Content-Type': CONTENT_TYPES[extname(name)] ?? 'application/octet-stream',
128-
'X-Content-Type-Options': 'nosniff',
129-
},
130-
})
141+
return await readFile(join(rootDir, name))
131142
} catch (error) {
132-
logger.error('Could not read a bundled page asset', { name, error: getErrorMessage(error) })
133-
return notFound()
143+
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
144+
logger.error('Could not read a bundled page asset', { name, error: getErrorMessage(error) })
145+
return null
146+
}
134147
}
135148
}
149+
logger.error('Bundled page asset is missing', { name })
150+
return null
136151
}
137152

138153
/**
139-
* The bundled `static/` directory. `__dirname` is `dist/` in every build, so
140-
* this holds inside the packaged asar as well as in an unpackaged checkout.
154+
* Where the pages and their assets live. `__dirname` is `dist/` in every
155+
* build, so `static/` resolves inside the packaged asar as well as in an
156+
* unpackaged checkout. The brand font is copied into `static/` only when
157+
* packaging (electron-builder.yml); an unpackaged run reads it from the web
158+
* app's public fonts instead, so nothing generated has to exist in the tree
159+
* and a cached build restores everything the pages need.
141160
*/
142-
function localPageRoot(): string {
143-
return join(__dirname, '..', 'static')
161+
function localPageRoots(): string[] {
162+
const roots = [join(__dirname, '..', 'static')]
163+
if (!app.isPackaged) {
164+
roots.push(join(__dirname, '..', '..', 'sim', 'public', 'brand', 'fonts'))
165+
}
166+
return roots
144167
}
145168

146169
/**
147170
* Serves the scheme on a session. Handlers are per session, so every partition
148171
* that hosts a bundled page installs one; repeat calls are no-ops.
149172
*/
150-
export function attachLocalPageProtocol(ses: Session, rootDir: string = localPageRoot()): void {
173+
export function attachLocalPageProtocol(
174+
ses: Session,
175+
rootDirs: readonly string[] = localPageRoots()
176+
): void {
151177
if (ses.protocol.isProtocolHandled(LOCAL_PAGE_SCHEME)) {
152178
return
153179
}
154-
ses.protocol.handle(LOCAL_PAGE_SCHEME, createLocalPageHandler(rootDir))
180+
ses.protocol.handle(LOCAL_PAGE_SCHEME, createLocalPageHandler(rootDirs))
155181
}

0 commit comments

Comments
 (0)