web: browse mode without wai-handler-launch - #2722
Merged
Merged
Conversation
The Windows branch ran a fixed Firefox path under Program Files, so it failed for anyone without Firefox there. openBrowserOn now tries the open-browser package first: ShellExecuteW through the Win32 API on Windows, `open` on mac, `xdg-open` on Linux and the BSDs, without waiting for the browser to exit. (Running rundll32 or `cmd /c start` through the process package is not an option: on Windows it quotes every argument, which rundll32 does not parse as intended.) On Linux the previous launchers, sensible-browser, gnome-www-browser and firefox, remain as fallbacks when xdg-open is not installed. A fallback that is not installed used to raise an IOException out of readProcessWithExitCode instead of falling through to the next one, so the chain never got past its first entry; it is now treated like a failed launch. hledger-web's browse mode is about to use this too. AI usage: Claude Fable 5.1, (estimate included in the next commit message)
The default --serve-browse mode ran through wai-handler-launch, which inserted an inline ping script into every HTML response (rewriting the body as a stream, gunzipping if needed) and exited the server when the pings stopped. Under the Content-Security-Policy that script could only be allowed by its sha256 hash, which meant pinning the library to one exact version. hledger-web now does the same job itself, in Main.hs: - warp is run with a setBeforeMainLoop hook; once it is listening, the browser is opened with openBrowserOn (from hledger), and an idle wait runs alongside warp under `race`, as the library did, so the function returns normally only on the idle exit and a warp failure still raises. - a WAI middleware answers /_ping with 204 No Content, before the app, and records the time (monotonic clock). - the idle wait exits two minutes after the last ping, so the "no browser windows were open for 2m" message is now exact; the library checked a flag every 120s and exited anywhere from 2 to 4 minutes after the last ping. The ping itself moves to hledger.js: on load, then every 30 seconds (the library pinged every 60s against the same 120s limit, so a single missed ping could end the session). The page is told to ping by a data-browse-mode attribute on the body, set by defaultLayout in browse mode only, so pages served with --serve never ping. With no inserted script, the policy's script hash and the version pin go away; async is added for `race`. The library also silenced warp's exception handler in browse mode; that is not kept, so browse mode now reports server exceptions the same way --serve does. Tests: the yesod tests check the body marker in browse mode and its absence with --serve; the browse-mode browser spec checks that the ping goes out and is answered. The manual's description of the exit now matches the code (requests other than the ping never counted). The ping is sent to the page's own origin rather than to the base url, because the policy allows requests to our origin only: a page reached at http://localhost:5000 when the base url says 127.0.0.1 would otherwise have its pings blocked and the server would exit under it. It is also the first thing the page's startup handler does, so that a failure later in that handler, eg in the chart code, cannot stop it. Windows: the library launched the browser through a C ShellExecute shim; hledger's launcher now does the same through the Win32 API via the open-browser package. The browser suite cannot run on Windows, so this path has not been exercised. AI usage: Claude Fable 5.1, ~60k output tokens (including a code review pass)
Member
|
Thanks, I see. It sounds good. It's probably also worth thinking about the auto-shutdown feature and if it's truly needed. The original motivation was for non-technical/GUI users, particularly on Windows, who would start hledger-web.exe by double clicking its icon, and would never terminate it; the auto shutdown aims to avoid wasting resources, and especially to prevent conflicts/problems caused by leaving stray processes running. Maybe there's some other way to meet the goals. |
Contributor
Author
|
based on: mature code that hasn't been touched in a long time. |
Contributor
Author
|
I think it is valuable functionality that is worth it. I am not sure that is true with sandstorm on the other hand, I haven't looked into that thoroughly, but I have questions. |
Contributor
Author
|
It's very good for me. I frequently want to open up a register filtered to a set of transactions that I want to inspect and it's nice that the server dies when I close the tab. |
Member
|
That's great to hear! Yours is the first feedback about it, in ~20 years. :-) As far as I know the Sandstorm app is very little used, and mostly unmaintained. I think we don't need to spend effort supporting it in 2.x. Unless Sandstorm is having a great resurgence. |
Fixes from a review pass over the previous two commits: - open-browser >=0.5 is not in the lts-22.44, lts-23.28 and lts-24.57 snapshots (they have 0.2.1.1 and 0.4.0.0), so the stack96, stack98 and stack910 builds, including the oldest CI job, could not resolve their plan. Add open-browser-0.5.1.0 to their extra-deps. (The bound cannot be relaxed: 0.5 is where xdg-open stopped blocking until the browser closed.) - openBrowserOn uses open-browser for Windows only, where it asks the Win32 API. Its unix backends start the launcher without waiting for it, so they report success whenever the program exists, and the fallbacks and the "please open your browser and visit" message could never be reached. On mac and Linux the launchers are run to completion with their exit code checked, as before, xdg-open first. - The idle wait allows one more ping interval when it finds it slept far longer than asked. On macOS and Windows the monotonic clock runs on through a system sleep, during which no page could ping, so a laptop woken after a long sleep would otherwise exit the server under an open page before its next ping. - serveAndBrowse takes the same warp settings as --serve, launching the browser from warp's before-main-loop hook rather than through an MVar, and sleeps with System.Time.Extra.sleep. - The ping goes to the base url's path on the page's own origin, so a proxy expecting a path prefix gets one, and it is the first thing the page's startup handler does, as its commit message already claimed. AI usage: Claude Fable 5.1, ~15k output tokens (including a code review pass)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The follow-up to #2720: hledger-web's default
--serve-browsemode no longer uses wai-handler-launch.Why. The library inserted an inline ping script into every HTML page, which the Content-Security-Policy could only allow by hash, with the package pinned to one exact version to keep the hash honest. Its browser launch on Windows also went through a C shim.
What replaces it. About 45 lines in
Main.hs, keeping the library's process shape (warp with a before-main-loop hook, browser opened once listening,racebetween warp and an idle wait, so a warp failure still raises and only the idle exit returns normally):/_pingwith 204 and stamps a monotonic clock, before the request reaches the app, as the library did;hledger.js: on load, then every 30s (the library pinged every 60s against the same limit, so one missed ping could end a session). It runs only whendefaultLayouthas marked the body withdata-browse-mode, which it does in browse mode only, so--servepages never ping. It goes to the page's own origin, not the base url, since the policy allows requests to our origin only;openBrowserOnin hledger now tries the open-browser package first (ShellExecuteW via the Win32 API on Windows,openon mac,xdg-openon Linux and the BSDs), then the old Linux fallbacks, skipping ones that are not installed. This fixes the Windows branch, which ran a fixed Firefox path, for--webmanand thehelppages too.The hash and the version pin leave the policy and
package.yaml; wai-handler-launch is replaced by open-browser (in hledger) and async (in hledger-web).Tests. The yesod tests check the body marker in browse mode and its absence with
--serve. The browser suite's browse-mode spec waits for the actual ping request and checks its 204, with the launcher stubbed on PATH. A timed run with the stub exited 120s after the last ping to the second. The Windows launch cannot be exercised here: the browser suite does not run on Windows, and CI's Windows job builds and unit-tests only.Follow-up.
--port 0is still refused in browse mode. That restriction (from #2559) existed only because the library needed a known port up front; with warp run by hledger-web itself it can be lifted, and I'll do that in a separate PR.AI usage: Claude Fable 5.1 throughout, ~60k output tokens (including a code review pass); both commit messages and this description were reviewed and edited by me before publishing.