mirror of
https://github.com/MODSetter/SurfSense.git
synced 2026-10-19 15:44:27 +00:00
Merge pull request #2203 from TonyWu2333/fix/script-traceback-relative-paths
fix(local): print a script's traceback relative to its run folder
This commit is contained in:
commit
c45d0fb7d6
3 changed files with 68 additions and 3 deletions
|
|
@ -113,7 +113,7 @@ Each version is its own artifact. v1 is its own root; a version made from anothe
|
|||
- **The process.** The worker in its `--run-document-script <folder>` mode, which loads no queue, database or settings: the frozen worker binary when packaged, `worker.py` on the backend's Python in development. The child changes into the folder and runs `script.py` as `__main__`; a failure prints the traceback without the runner's own frames and exits 1.
|
||||
- **Its environment** is built, not inherited: `PATH`, plus `SYSTEMROOT`, `WINDIR`, `TEMP` and `TMP` on Windows or `HOME` and `LANG` elsewhere, then `OUTPUT_PATH`, `IMAGES_DIR`, `TEMPLATE_PATH` only when the run has a template, `MPLBACKEND=Agg`, `MPLCONFIGDIR` inside the folder, `PYTHONUTF8=1` and `PYTHONDONTWRITEBYTECODE=1`. No key, secret or `OPENCODE_*` variable reaches it.
|
||||
- **The limit** is 120 seconds, and a cancelled job stops its script within a second. A cancel check that finds the database locked past SQLite's busy wait skips that look rather than fail a script with nothing wrong. Either way the script and everything it started are killed: on Windows a job object holds them from the start and dies with the worker too; elsewhere the script leads its own process group, which it kills when the worker's end of its stdin closes.
|
||||
- **The result** is the file's bytes, or one line the agent can act on ("SyntaxError: …", "timed out after 120 s", "the script wrote no file at OUTPUT_PATH") and the last 30 lines of the traceback. The worker keeps only the last 64 KiB of the script's stderr, which a runaway script could fill with gigabytes. Its stdout is drained too, so a script that prints a lot never blocks; a document script's is dropped.
|
||||
- **The result** is the file's bytes, or one line the agent can act on ("SyntaxError: …", "timed out after 120 s", "the script wrote no file at OUTPUT_PATH") and the last 30 lines of the traceback. The worker keeps only the last 64 KiB of the script's stderr, which a runaway script could fill with gigabytes. Paths in the run folder are written relative to it, anywhere in that stderr: a warning's, a frame's, or one in an exception message, which on Windows has its backslashes doubled. Its stdout is drained too, so a script that prints a lot never blocks; a document script's is dropped.
|
||||
|
||||
**Analysis runs.** `surfsense_analyze_data` ([agent](agent.md#analyses)) queues `analyse_data` on the Studio queue at priority 100, ahead of Studio's own jobs, dropped if not started within 60 seconds, and waits for its Huey result ([`data_analysis/task.py`](../../surfsense_local/backend/modules/agent/data_analysis/task.py)). It runs in the Studio worker because the packaged worker is the binary that ships pandas. [`analysis_run.py`](../../surfsense_local/backend/worker/document_script/analysis_run.py) gives the script a run folder with `input/`, holding a read-only copy of each source's original file under its own name (a second file of the same name gains ` [<id>]`) and `manifest.json`, which lists each copy's `file`, `document_id` and `title`, and an empty `output/`; the environment has `INPUT_DIR` and `OUTPUT_DIR` in place of `OUTPUT_PATH` and `IMAGES_DIR`, the rest as above. It keeps the first 64 KiB of stdout and counts the rest. On success the files at the top of `output/` move into the thread's `outputs/analysis/<run>/` ([`kept_outputs.py`](../../surfsense_local/backend/worker/document_script/kept_outputs.py)): only `.csv`, `.tsv`, `.png`, `.jpg`, `.jpeg`, `.svg`, `.xlsx`, `.json` and `.txt`, never Markdown, which opencode could read as instructions, at most 20, each at most 25 MB, named with letters, digits, `_` and `-` and at most 40 characters before the suffix. Anything else is named as not kept. A failed run keeps nothing. The folder goes afterwards, its read-only copies made writable first.
|
||||
|
||||
|
|
@ -319,7 +319,6 @@ Every pipeline returns a `Built`: a `title`, the `markdown` that is always the i
|
|||
- The PDF builder keeps Helvetica for Latin-1 and sets other letters in DejaVu Sans (shipped with matplotlib) or, for Chinese, Japanese and Korean, ReportLab's CID fonts, which are not embedded and rely on the viewer's substitutes. Reading one back has the same dependence: on a computer with no CJK font, pdfium cannot measure a glyph of those fonts and drops a text run with no extent, so a run of one such character is lost and `第1章 概要` reads back as `1 概要`. Studio keeps the Markdown as this builder's body, so that reaches the index only through a script's PDF that uses those fonts, whose body is read back from the file. Arabic and Hebrew are drawn in logical order and Arabic unjoined, and Devanagari and other scripts that need shaping render as boxes: both need a bidi and shaping library the app does not carry. A chart whose labels the matplotlib font lacks (Chinese, for one) is shown as a table.
|
||||
- The Word builder's numbered lists use the template's `List Number` style, whose numbering runs on across lists rather than restarting at 1.
|
||||
- A document or analysis script runs with the user's privileges: the runner gives it a clean environment and a time limit, but it can read any file the user can and open network connections ([ADR 0039](../adr/0039-document-scripts-run-without-approval.md)).
|
||||
- A failed script's stored reason repeats the run folder's absolute path in every traceback frame, about 130 characters each against its 500, so a long traceback keeps few of its lines for the agent.
|
||||
- Without Office support, a workbook's formulas show the value the script stored with them, since neither the runner nor Studio's viewer recalculates: xlsxwriter stores 0 unless given one, which the skill tells the agent to pass. Excel recalculates when it opens the file. The render result says the values were not recalculated.
|
||||
- Only the agent's workbooks are recalculated; Studio's own Spreadsheet format is not.
|
||||
- A revised copy is cut from its design ([03](../proposals/file-agent/03-editable-artifacts.md)): versions are artifacts, not `artifact_versions` rows, and engines run in the Studio worker's thread, not an engines queue's child process; the agent anchors edits by quote, with no `inspect_document` ids; there are no partial workflows, no per-change accept or reject and no side list of changes, no "With changes (internal)" download and no leak scan, no review record, no reconcile when the original changes, and no "Make a revised copy" in the source viewer. Word headers, footers and footnotes are not edited. An edited workbook is not recalculated until Excel opens it, and a deck has no thumbnails or diff. The report's codes reach the user in English through the agent: the interface does not render them.
|
||||
|
|
|
|||
|
|
@ -133,6 +133,52 @@ def test_a_script_that_raises_returns_its_error_and_traceback() -> None:
|
|||
assert "runpy" not in result.traceback_tail
|
||||
|
||||
|
||||
def test_a_traceback_names_the_script_without_its_run_folder(data_dir: Path) -> None:
|
||||
"""The folder is gone once the run ends, and its absolute path in every frame
|
||||
would crowd the traceback's lines out of the stored reason."""
|
||||
script = "def build():\n raise ValueError('no table rows')\n\nbuild()\n"
|
||||
|
||||
result = run_document_script(script, output_name="document.docx", images={})
|
||||
|
||||
assert result.traceback_tail is not None
|
||||
assert 'File "script.py", line 2, in build' in result.traceback_tail
|
||||
assert _scripts_root(data_dir).name not in result.traceback_tail
|
||||
|
||||
|
||||
def test_a_missing_file_is_named_without_its_run_folder(data_dir: Path) -> None:
|
||||
"""An exception message holds the contract's absolute path, on Windows as a
|
||||
repr with its backslashes doubled."""
|
||||
script = (
|
||||
"import os\nopen(os.path.join(os.environ['IMAGES_DIR'], 'nope.png'), 'rb')\n"
|
||||
)
|
||||
|
||||
result = run_document_script(script, output_name="document.docx", images={})
|
||||
|
||||
assert result.error is not None
|
||||
assert result.error.startswith("FileNotFoundError")
|
||||
assert "nope.png" in result.error
|
||||
assert _scripts_root(data_dir).name not in result.error
|
||||
assert result.traceback_tail is not None
|
||||
assert _scripts_root(data_dir).name not in result.traceback_tail
|
||||
|
||||
|
||||
def test_a_warning_before_the_traceback_names_the_script_without_its_run_folder(
|
||||
data_dir: Path,
|
||||
) -> None:
|
||||
"""A warning printed before the raise starts with the script's absolute path."""
|
||||
script = (
|
||||
"import warnings\n"
|
||||
"warnings.warn('old call', FutureWarning)\n"
|
||||
"raise ValueError('no table rows')\n"
|
||||
)
|
||||
|
||||
result = run_document_script(script, output_name="document.docx", images={})
|
||||
|
||||
assert result.traceback_tail is not None
|
||||
assert "script.py:2: FutureWarning: old call" in result.traceback_tail
|
||||
assert _scripts_root(data_dir).name not in result.traceback_tail
|
||||
|
||||
|
||||
def test_a_syntax_error_names_itself() -> None:
|
||||
"""A script that does not compile fails with the SyntaxError, not a crash."""
|
||||
result = run_document_script(
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ A process, not a thread: only a process can be stopped at its time limit (ADR 00
|
|||
"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
|
|
@ -127,11 +128,30 @@ def run_child(
|
|||
exit_code=exit_code,
|
||||
stdout=printed.replace("\r\n", "\n"),
|
||||
stdout_cut=stdout.dropped,
|
||||
stderr=stderr.text(timeout_seconds=LAST_OUTPUT_SECONDS),
|
||||
stderr=without_the_run_folder(
|
||||
stderr.text(timeout_seconds=LAST_OUTPUT_SECONDS), folder
|
||||
),
|
||||
seconds=seconds,
|
||||
)
|
||||
|
||||
|
||||
def without_the_run_folder(text: str, folder: Path) -> str:
|
||||
"""Paths in the run folder, relative to it: the folder is gone once the run
|
||||
ends, and its absolute path in every line crowds the traceback out of the
|
||||
500-character reason the agent reads. A Windows path in an exception message
|
||||
is a repr, with its backslashes doubled."""
|
||||
relative: dict[str, str] = {}
|
||||
for path in {str(folder), str(folder.resolve())}:
|
||||
for written, rewritten in ((os.path.join(path, ""), ""), (path, ".")):
|
||||
relative[written] = rewritten
|
||||
relative[written.replace("\\", "\\\\")] = rewritten
|
||||
# Longest first: the resolved form can end with the given one, as
|
||||
# /private/var/... does /var/... on macOS.
|
||||
for prefix in sorted(relative, key=len, reverse=True):
|
||||
text = text.replace(prefix, relative[prefix])
|
||||
return text
|
||||
|
||||
|
||||
def _wait_for_exit(process: subprocess.Popen, timeout_seconds: float) -> int | None:
|
||||
deadline = time.monotonic() + timeout_seconds
|
||||
while (remaining := deadline - time.monotonic()) > 0:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue