Skip to content

Commit 5861494

Browse files
ralyodioclaude
andauthored
fix(launcher): never replace a profile state file we could not read (#77)
* fix(launcher): never replace a profile state file we could not read A profile that had worked for months came up unusable: a blank side panel, a New Tab that never finished loading. A fresh profile on the same machine, same build, same engine, with the same extension loaded, was fine — so the profile was the fault, not the code it ran. Three blocks here edit Chromium's JSON state. Every one of them did: try: d = json.load(open(p)) if os.path.exists(p) else {} except Exception: d = {} ... json.dump(d, open(p, "w")) Both halves are wrong, and they feed each other. The write is in-place and truncating, so an interrupted launch leaves a half-written file. The read then treats that file as absent and writes a stub holding only the key that block cared about. Default/Preferences IS the profile — search engine, startup, every extension's state — so the second launch after an interrupted one silently factory-resets it, and nothing says so. So: a state file that exists and does not parse is now left exactly as found, with a line on stderr naming it. Refusing to write is not refusing to start — the browser still launches, just without that block's setting applied. And all three writes now land as a rename (fsync, then os.replace) instead of in place, so there is no longer a truncated file for the next launch to misread. #75 fixed the write half for the search block alone and left its read, and left the other two blocks untouched; this finishes the job. Tests: 5 cases — a corrupt Preferences and a corrupt Local State survive a launch byte-for-byte, the refusal is announced, the browser still starts, and setting restore_on_startup keeps every unrelated key. Verified 3 of the 5 fail against the pre-fix launcher. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ci): drop a committed node_modules symlink, and ignore it properly `pnpm install --frozen-lockfile` failed on this branch with ENOENT trying to mkdir apps/desktop/node_modules — because the path was checked in as a symlink pointing at an absolute path that exists on no runner. It got committed because .gitignore said `node_modules/`, and a trailing slash matches directories only. A symlink named node_modules — which is what running the suite against a hoisted store leaves behind — is not a directory, so it was never ignored. Dropping the slash covers both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5a75862 commit 5861494

3 files changed

Lines changed: 138 additions & 18 deletions

File tree

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
node_modules/
1+
node_modules
22
dist/
33
build/
44
out/

apps/desktop/launcher/tronbrowser

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -170,19 +170,42 @@ fi
170170
# "Always prompt for install". Pre-seed it in the profile's Local State so the
171171
# bundled chromium-web-store can install extensions. Best-effort (needs python3).
172172
if command -v python3 >/dev/null 2>&1; then
173-
TB_LOCAL_STATE="$DATA/Local State" python3 - <<'PY' 2>/dev/null || true
174-
import json, os
173+
TB_LOCAL_STATE="$DATA/Local State" python3 - <<'PY' || true
174+
import json, os, sys, tempfile
175175
p = os.environ["TB_LOCAL_STATE"]
176176
os.makedirs(os.path.dirname(p), exist_ok=True)
177-
try:
178-
d = json.load(open(p)) if os.path.exists(p) else {}
179-
except Exception:
177+
# An existing file that does not parse is LEFT ALONE. Replacing it with a fresh
178+
# dict is how a profile gets wiped: this file holds every setting in the profile,
179+
# so a stub is indistinguishable from a factory reset — and the usual reason it
180+
# does not parse is a truncating write like the one this block used to do.
181+
if os.path.exists(p):
182+
try:
183+
with open(p) as f:
184+
d = json.load(f)
185+
except Exception:
186+
sys.stderr.write("TronBrowser: %s did not parse — leaving it untouched.\n" % p)
187+
raise SystemExit(0)
188+
else:
180189
d = {}
181190
exp = d.setdefault("browser", {}).setdefault("enabled_labs_experiments", [])
182191
flag = "extension-mime-request-handling@2"
183192
if flag not in exp:
184193
exp.append(flag)
185-
json.dump(d, open(p, "w"))
194+
# Land it as a rename. This file is the profile; a truncated in-place write
195+
# loses every setting in it, and the next launch then reads it as unparseable.
196+
fd, tmp = tempfile.mkstemp(dir=os.path.dirname(p))
197+
try:
198+
with os.fdopen(fd, "w") as f:
199+
json.dump(d, f)
200+
f.flush()
201+
os.fsync(f.fileno())
202+
os.replace(tmp, p)
203+
except Exception:
204+
try:
205+
os.unlink(tmp)
206+
except OSError:
207+
pass
208+
raise
186209
PY
187210
fi
188211

@@ -240,16 +263,25 @@ if command -v python3 >/dev/null 2>&1 && [ "$SEARCH_PREV" != "$SEARCH_WANT" ]; t
240263
TB_ENGINE="$(search_engine_spec "$SEARCH_WANT")" \
241264
TB_PREV_URL="$SEARCH_PREV_URL" \
242265
TB_FORCE="$SEARCH_EXPLICIT" \
243-
python3 - <<'PY' 2>/dev/null || true
244-
import json, os, tempfile
266+
python3 - <<'PY' || true
267+
import json, os, sys, tempfile
245268
p = os.environ["TB_PREFS"]
246269
name, keyword, url = os.environ["TB_ENGINE"].split("|")
247270
prev_url = os.environ.get("TB_PREV_URL", "")
248271
force = os.environ.get("TB_FORCE") == "1"
249272
os.makedirs(os.path.dirname(p), exist_ok=True)
250-
try:
251-
d = json.load(open(p)) if os.path.exists(p) else {}
252-
except Exception:
273+
# An existing file that does not parse is LEFT ALONE. Replacing it with a fresh
274+
# dict is how a profile gets wiped: this file holds every setting in the profile,
275+
# so a stub is indistinguishable from a factory reset — and the usual reason it
276+
# does not parse is a truncating write like the one this block used to do.
277+
if os.path.exists(p):
278+
try:
279+
with open(p) as f:
280+
d = json.load(f)
281+
except Exception:
282+
sys.stderr.write("TronBrowser: %s did not parse — leaving it untouched.\n" % p)
283+
raise SystemExit(0)
284+
else:
253285
d = {}
254286
255287
# Only ever overwrite an engine that is absent or that we put there ourselves.
@@ -284,16 +316,39 @@ fi
284316
# goes through the new-tab path, which honors the chrome_url_overrides feed. Once
285317
# per profile, then respect the user's chrome://settings/onStartup choice.
286318
if command -v python3 >/dev/null 2>&1 && [ ! -f "$DATA/.tron-startup-ntp" ]; then
287-
TB_PREFS="$DATA/Default/Preferences" python3 - <<'PY' 2>/dev/null || true
288-
import json, os
319+
TB_PREFS="$DATA/Default/Preferences" python3 - <<'PY' || true
320+
import json, os, sys, tempfile
289321
p = os.environ["TB_PREFS"]
290322
os.makedirs(os.path.dirname(p), exist_ok=True)
291-
try:
292-
d = json.load(open(p)) if os.path.exists(p) else {}
293-
except Exception:
323+
# An existing file that does not parse is LEFT ALONE. Replacing it with a fresh
324+
# dict is how a profile gets wiped: this file holds every setting in the profile,
325+
# so a stub is indistinguishable from a factory reset — and the usual reason it
326+
# does not parse is a truncating write like the one this block used to do.
327+
if os.path.exists(p):
328+
try:
329+
with open(p) as f:
330+
d = json.load(f)
331+
except Exception:
332+
sys.stderr.write("TronBrowser: %s did not parse — leaving it untouched.\n" % p)
333+
raise SystemExit(0)
334+
else:
294335
d = {}
295336
d.setdefault("session", {})["restore_on_startup"] = 5 # 5 = open the New Tab page
296-
json.dump(d, open(p, "w"))
337+
# Land it as a rename. This file is the profile; a truncated in-place write
338+
# loses every setting in it, and the next launch then reads it as unparseable.
339+
fd, tmp = tempfile.mkstemp(dir=os.path.dirname(p))
340+
try:
341+
with os.fdopen(fd, "w") as f:
342+
json.dump(d, f)
343+
f.flush()
344+
os.fsync(f.fileno())
345+
os.replace(tmp, p)
346+
except Exception:
347+
try:
348+
os.unlink(tmp)
349+
except OSError:
350+
pass
351+
raise
297352
PY
298353
mkdir -p "$DATA"; : > "$DATA/.tron-startup-ntp"
299354
fi

apps/desktop/test/launcher.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,71 @@ describe('omnibox search engine', () => {
226226
});
227227
});
228228

229+
// --- Damaged profile ---------------------------------------------------------
230+
// Three blocks in the launcher edit Chromium's JSON state files. Every one of
231+
// them used to do `except Exception: d = {}` and then write the result — so a
232+
// file that failed to parse was REPLACED by a stub holding only the key that
233+
// block cared about. Preferences is the whole profile, which made that a silent
234+
// factory reset, and the usual reason it failed to parse was the non-atomic
235+
// write the same block did on the previous launch. These tests pin the rule:
236+
// a state file we cannot read is a file we do not touch.
237+
238+
const localStatePath = (home: string) => join(home, 'profile', 'Local State');
239+
240+
/** A profile whose JSON state files are present but corrupt. */
241+
function seedCorrupt(which: 'prefs' | 'localstate'): { home: string; path: string } {
242+
const home = mkdtempSync(join(tmpdir(), 'tron-launcher-'));
243+
homes.push(home);
244+
const path = which === 'prefs' ? prefsPath(home) : localStatePath(home);
245+
mkdirSync(dirname(path), { recursive: true });
246+
// What a truncated write leaves behind: valid JSON's opening, then nothing.
247+
writeFileSync(path, '{"default_search_provider_data": {"template_ur');
248+
return { home, path };
249+
}
250+
251+
describe('damaged profile', () => {
252+
it('leaves an unparseable Preferences exactly as it found it', () => {
253+
const { home, path } = seedCorrupt('prefs');
254+
const before = readFileSync(path, 'utf8');
255+
run([], { home });
256+
expect(readFileSync(path, 'utf8')).toBe(before);
257+
});
258+
259+
it('leaves an unparseable Local State exactly as it found it', () => {
260+
const { home, path } = seedCorrupt('localstate');
261+
const before = readFileSync(path, 'utf8');
262+
run([], { home });
263+
expect(readFileSync(path, 'utf8')).toBe(before);
264+
});
265+
266+
it('says which file it refused to touch, rather than failing silently', () => {
267+
const { home } = seedCorrupt('prefs');
268+
const { stderr } = run([], { home });
269+
expect(stderr).toContain('did not parse');
270+
});
271+
272+
it('still launches the browser when a state file is unreadable', () => {
273+
// Refusing to write must not become refusing to start.
274+
const { home } = seedCorrupt('prefs');
275+
const { argv } = run([], { home });
276+
expect(valueOf(argv, '--user-data-dir')).toEqual([join(home, 'profile')]);
277+
});
278+
279+
it('keeps unrelated settings when it sets the startup page', () => {
280+
// The restore_on_startup block rewrites the whole file to change one key.
281+
const home = mkdtempSync(join(tmpdir(), 'tron-launcher-'));
282+
homes.push(home);
283+
const path = prefsPath(home);
284+
mkdirSync(dirname(path), { recursive: true });
285+
writeFileSync(path, JSON.stringify({ bookmark_bar: { show_on_all_tabs: true }, extensions: { settings: { abc: 1 } } }));
286+
run([], { home });
287+
const json = JSON.parse(readFileSync(path, 'utf8'));
288+
expect(json.session?.restore_on_startup).toBe(5);
289+
expect(json.bookmark_bar?.show_on_all_tabs).toBe(true);
290+
expect(json.extensions?.settings?.abc).toBe(1);
291+
});
292+
});
293+
229294
describe('engine reporting', () => {
230295
it('names the engine it is about to run', () => {
231296
const { stderr } = run([], { version: 'Chromium 141.0.0.0' });

0 commit comments

Comments
 (0)