Skip to content

Resolve all four file-validation bugs from #135#136

Merged
stevenweaver merged 2 commits into
mainfrom
fix/file-validation-bugs-135
Jun 8, 2026
Merged

Resolve all four file-validation bugs from #135#136
stevenweaver merged 2 commits into
mainfrom
fix/file-validation-bugs-135

Conversation

@stevenweaver

@stevenweaver stevenweaver commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

Addresses all four bugs from #135 — fixes the actual user-facing failures (Bugs 3 + 4) and the two telemetry gaps that surfaced them (Bugs 1 + 2).

Bug 1 — Unknown errors missing message field (+page.svelte)

338 of 342 `unknown` file-validation-error events had no `message` field in telemetry because `errorMsg = error.message || ''` was empty when something threw without a string. Replace the empty-string fallback with a defensive chain (`error.message → error.name → error.toString() → sentinel`) so the analytics payload always has something to work with.

Bug 2 — HTML responses parsed as JSON (WasmAnalysisRunner.js)

~290 `analysis-start-blocked` events surfaced raw V8 parse errors like `Unexpected token '<', '<!doctype'... is not valid JSON`. The actual culprit was `WasmAnalysisRunner.js:448` calling `safeParseJSON` without first checking whether `fetch(jsonBlob)` had returned HTML (which happens when HyPhy fails to produce a result file and the path resolves to a 404 page). Mirror the existing guard from `+page.svelte:805` so the user sees an actionable message instead of a parser error.

(The issue suggested `BackendAnalysisRunner.js` was at fault, but the backend uses socket.io — there's no `response.json()` there to guard. The real source was the WASM result fetch.)

Bug 3 — Neighbor-joining tree blocked without fallback (AnalyzeTab.svelte)

177 `analysis-start-blocked` events with `No neighbor-joining tree available. Please select a different tree source or upload a tree.` Root cause: `selectedTreeSource` defaults to `'inferred'`, but if datareader didn't produce an NJ tree (`FILE_INFO.nj` missing), the TreeSourceSelector disables the radio button while the underlying state stays `'inferred'`. Users then click Run and `getSelectedTreeData()` returns empty.

Add a reactive guard that switches `selectedTreeSource` to whichever source is actually available (`'uploaded'` if present, otherwise `'upload-new'`). Also tighten the error message when no fallback exists so users understand what to do.

Bug 4 — Genetic code parameter receiving invalid values (MethodSelector.svelte + WasmAnalysisRunner.js)

19+ `'X' is not a valid value for parameter 'Choose Genetic Code'` events with values like `Vertebrate`, `All`, `Source`, `0.1`. Root cause: aioli's `exec()` does a plain `command.split(' ')` with no quote awareness (confirmed by reading the aioli worker source). Multi-word genetic code names like `'Vertebrate mitochondrial'` get split into `--code Vertebrate` + a stray `mitochondrial`, which causes HyPhy to reject `Vertebrate` and then consume the next option's value as a fallback — explaining `'All'` (from `--branches All`), `'0.1'` (from `--pvalue 0.1`), and `'Source'` (from RELAX branch labels).

Pass the numeric `geneticCodeId` (always single-token) as `--code` instead of the descriptive name. The backend already does this via socket params.

Test plan

  • All 201 existing tests pass
  • Smoke test: trigger a HyPhy failure that produces no result file → confirm "HyPhy X analysis did not produce a result file" toast instead of the parser error
  • Smoke test: throw something without a message string → confirm `file-validation-error` events include a `message` field
  • Smoke test: upload an alignment where NJ inference fails → confirm `selectedTreeSource` auto-switches and the error message is helpful
  • Smoke test: run a method with a multi-word genetic code (e.g. "Vertebrate mitochondrial") → confirm HyPhy now receives the numeric code and accepts it

Follow-ups (out of scope)

  • `WasmAnalysisRunner.js` builds args as a space-joined string. The space-split issue affects more than just `--code` — any arg with a multi-word value (file paths, branch labels) is vulnerable. A larger refactor to pass `args` as an array to `aioli.exec(command, argsArray)` would eliminate the entire class of bug.
  • Bug 3's deeper question is why the NJ tree fails to build for some inputs. The current fix routes around the problem; understanding the root cause (small alignments? non-standard characters? specific datatypes?) would be its own investigation.

…etry

Two diagnosis fixes from issue #135.

Bug 2: ~290 analysis-start-blocked events surfaced raw V8 parse errors
like "Unexpected token '<', '<!doctype'..." because safeParseJSON was
being called on the output of fetch(jsonBlob) without first checking
whether the response was HTML. When HyPhy fails to produce a result
file the cliObj.download() path resolves to a 404 HTML page; throwing
a clear message gives the user actionable feedback and stops the parser
noise in telemetry.

Bug 1: 338 of 342 unknown file-validation-error events had no message
field because error.message was falsy. Fall back through error.name and
error.toString() before resorting to a sentinel string so the analytics
payload always has something to work with.
Bug 3: When the datareader doesn't produce an NJ tree (FILE_INFO.nj
missing), the TreeSourceSelector correctly disables the 'inferred'
radio but the underlying selectedTreeSource state stays 'inferred' (the
default). Users then click Run and hit "No NJ tree available". Add a
reactive guard that switches the state to whichever source is actually
available, and tighten the error message when no fallback exists.

Bug 4: Aioli's exec() does a plain command.split(' '), so multi-word
genetic code names like 'Vertebrate mitochondrial' get split into
'--code Vertebrate' + a stray 'mitochondrial' token, which causes HyPhy
to reject 'Vertebrate' and then consume the next option's value as a
fallback ('All' from --branches, '0.1' from --pvalue, 'Source' from
branch labels). Pass the numeric geneticCodeId (always single-token)
instead — the backend already does this.
@stevenweaver stevenweaver changed the title Guard WASM result parsing against HTML and harden unknown-error telemetry Resolve all four file-validation bugs from #135 Jun 8, 2026
@stevenweaver
stevenweaver merged commit 4a9e0f9 into main Jun 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant