case 4690 - report browser download/launch failures: logged at Error, specific error returned to the client
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -118,7 +118,7 @@ in this branch or a follow-up release. UPDATE: John has decided these all go in
|
||||
- [x] **Backup `pg_dump` call.** Fixed (Sockeye 8.1.0 still has the old call): new `RunProgram.RunNoShell` (no shell, `ArgumentList`, per-process environment, both streams read together, timeout kills the process tree, non-zero exit code returned as an error); backup passes `--host/--port/--username/--dbname/--format=custom/--file/--no-password` with the password in `PGPASSWORD`. The old `RunProgram.Run` now has no callers (see Deferred). Confirmed: `CoreJobBackup.cs:68` builds `postgresql://user:password@host:port/db` with the raw password, and `:94` adds `-Fc > file`; `RunProgram` runs it through `cmd.exe /C` (`RunProgram.cs:55`) or `/bin/sh -c` (`:82-89`). A password with spaces or `@ : / ? # %` breaks backups. Also, on Linux stderr is not redirected (`RunProgram.cs:86-92`), so a failed pg_dump returns empty output and is **reported as a successful backup** (`CoreJobBackup.cs:97`). Fix: password in `PGPASSWORD` in the process environment, connection parameters as separate arguments through `ProcessStartInfo.ArgumentList` (no shell), output with `-f`, stderr and exit code checked
|
||||
- [x] **Drop-and-recreate database.** Fixed: before the `DROP`, `DropAndRecreateDbAsync` checks `rolcreatedb OR rolsuper` for the current user; without it, boot stops with an E1000 message naming the user and the setting, and the database is untouched. Confirmed: `DbUtil.cs:342-345` drops, then creates; triggered by `AYANOVA_PERMANENTLY_ERASE_DATABASE` (`Startup.cs:567-570`). Without `CREATEDB` it drops the database and then fails to create it. Test the permission before dropping, or change the order
|
||||
- [x] **Report browser download location.** Fixed: downloads into `<AYANOVA_DATA_PATH>/report-browser` (derived `ServerBootConfig.REPORT_RENDER_BROWSER_DOWNLOAD_PATH`, not a new setting; empty when no data path is set, then beside the app as before) through `BrowserFetcherOptions.Path`, and launches the downloaded executable explicitly. `AYANOVA_REPORT_RENDER_BROWSER_PATH` still takes precedence. Existing installs keep an unused old download beside the app until it is removed. Confirmed: `new BrowserFetcher()` with the default path beside the app (`ReportBiz.cs:800`). Point it under the data directory in code. Beside the app it is read-only under a hardened systemd unit (`ProtectSystem=strict`), and a redeploy that replaces the app folder deletes it
|
||||
- [ ] **Report failures: return a specific error.** The Sockeye crash is n/a: rendering runs as a job, and `JobsBiz.ProcessJobAsync` catches every exception, logs it through NLog and marks the job Failed (`JobsBiz.cs:390-439`), so the process survives. What remains: a browser download or launch failure (`ReportBiz.cs:800`, `:838`, outside the render `try`) reaches the client only as a generic job failure. Catch it in `DoRenderJob` and return a render error that says what failed
|
||||
- [x] **Report failures: return a specific error.** Done: download and launch each in their own `try`; on failure `HandleBrowserFailure` logs at Error (was Debug via the job processor) and writes `rendererror.exception` ("Report rendering browser could not be downloaded to … / started from … with arguments …: (reason)"), which the client already shows as `JobFailed: …`; no client change. The Sockeye crash is n/a: rendering runs as a job, and `JobsBiz.ProcessJobAsync` catches every exception, logs it through NLog and marks the job Failed (`JobsBiz.cs:390-439`), so the process survives. What remains: a browser download or launch failure (`ReportBiz.cs:800`, `:838`, outside the render `try`) reaches the client only as a generic job failure. Catch it in `DoRenderJob` and return a render error that says what failed
|
||||
- [ ] **Data-list date filters, small fixes.** Confirmed: `>=` (`:790-793`) and `<` (`:795-798`) use `MaxSeconds` and should use `ZeroSeconds`; date `NotEqual` (`:806-814`) uses `MaxSeconds` on both sides and should exclude the whole minute (`< ZeroSeconds OR > MaxSeconds`). `DataListField` is dereferenced at `:24` and `:27` before the DEBUG-only null check at `:33`, so **release builds** throw a NullReferenceException on an unknown column, not only DEBUG builds
|
||||
- [ ] **Relative date tokens use the server's time zone** (`*today*`, `*yesterday*` …). Confirmed: `ClientToday = clientTimeStamp.Date` (`:399-400`) has `Kind=Unspecified`, and `ToUniversalTime()` (`:1060`, `:1065`) converts it with the server's zone. Wrong for any user in a different zone from the server. The fix builds the range from `clientTimeStamp.Offset`, with care for ranges that cross a DST change. **This changes behaviour** and affects the integration tests' `TIME_ZONE_ADJUSTMENT`. Probably better as a follow-up release
|
||||
|
||||
@@ -210,3 +210,4 @@ Found during the port, out of scope. One line each.
|
||||
| 2026-10-22 | Backup `pg_dump` without a shell; password in `PGPASSWORD` | On-demand backups through the API into a scratch folder with a space in its name, checked with `pg_restore --list`. Old code as `postgres`: 803 entries. New code as `postgres`: 803 entries. Failing stand-in `pg_dump` (exit code 2): reported as `BACKUP ERROR`, no file, no password in the log. Temporary superuser role `"AyaNova"` (named like the database) with password `p @:/?#% x`: old code failed (`too many command-line arguments (first is "@:/?#%")`), left a 0-byte backup file and echoed part of the password into the log; new code 803 entries, no errors. Role dropped afterwards. Integration suite = baseline. Linux not tested here: the stderr gap there is closed by the same code (no shell, exit code checked) |
|
||||
| 2026-10-22 | Drop-and-recreate: CREATEDB check before the drop | Throwaway non-superuser role `ayatest_nocreatedb` owning throwaway database `ayatest_erase`, server data path in scratch (the erase also clears the attachments folder). Without CREATEDB: boot stops with the new E1000 message, database still there. Old sequence by hand as the same role: `DROP DATABASE` succeeds, `CREATE DATABASE` fails "permission denied", database gone. With CREATEDB: erased and recreated (oid 145595 → 145596), schema 20 built, boot complete. Database and role dropped afterwards. Integration suite not rerun: the change is only reached with the erase setting |
|
||||
| 2026-10-22 | Report browser downloaded under the data path | Browsers in `bin` moved aside; server with an empty scratch data folder. 22/22 reports render; Chrome and ChromeHeadlessShell 155 downloaded into `<data>/report-browser`, launched from there (logged), nothing written to `bin`. Second run: first render 1.6 s (was 25 s with the download), no new files: download reused |
|
||||
| 2026-10-22 | Specific error for report browser download / launch failures | Download blocked (a file named `report-browser` in the data folder): job failed with `rendererror.exception` "…could not be downloaded to "…\report-browser" (check internet access and that the folder is writable): … Cannot create …", Error in the log, server still up. Launch with `AYANOVA_REPORT_RENDER_BROWSER_PATH` pointing nowhere: "…could not be started from "C:\no-such-folder\chrome.exe" with arguments '--no-sandbox --lang=en-CA' …: Browser was not found…", server still up. Normal: 22/22 reports, page counts = PuppeteerSharp 17; integration suite = baseline |
|
||||
|
||||
Reference in New Issue
Block a user