File-to-DB import should stop at the first bad file and tell the user which file and why

Abierto
  • ¿Algo no está bien?
#1,486 1 comentario 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
5/5
Tiempo estimado
Más de una semana
Aptitud para principiantes
35/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
clojure
Área
backend, database

Línea de trabajo

The report names imports.cljs, logseq.db/transact-sync, the dry-run scan wrapper, and the bb dev:import headless path. Start by tracing how a scan failure travels from the importer through the db worker to abort-file-graph-import!, including how the cause is logged and shown. Done means the first failure reports its relative file path and actionable cause, reaches the renderer without restarting the worker, and leaves no partial graph; the issue also references #1469 for cleanup behavior.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Search first

I searched logseq/db-test for the toast text ("Scanning the folder failed") and for import-failure wording and found no issue about the error reporting itself. #1469 is one specific failure of this kind.

What Happened?

Importing a real, multi-year OG graph with File to DB graph failed with a toast that only says Unexpected error: Scanning the folder failed. It gives no file name, no block and no reason. Re-running gave the same message each time, so there was nothing to act on.

I had to reproduce the failure headlessly to learn what was wrong. Working through that graph exposed a series of independent problems, each in a different file, and every one surfaced as the same generic message in the app. The detail is lost at several points:

  • The dry-run scan wraps whatever failed in (ex-info "Scanning the folder failed" {:code :import-scan/failed} cause). The toast shows only the outer message, so the cause is dropped.
  • The worker logs transact-failed [object Object], because the rejected value is not an Error and is printed with str.
  • After a failed transaction in the scan the db-worker process restarts. The renderer sees POST /v1/invoke net::ERR_EMPTY_RESPONSE, then :db-worker-runtime-recovering and :db-worker-runtime-recovered :worker :replaced. This looks like an unhandled promise rejection ending the worker, so even the real error never reaches the UI.
  • The headless importer (bb dev:import) is better, because it prints Import failed on "<path>" with error: …. But the messages are internal (No ident found for :clock-time, Expected number or lookup ref for entity id, got nil) and mean nothing to someone with a notes folder.

I am not asking the importer to learn to handle every one of these inputs. The ask is narrower: when it cannot import something, it should fail clearly, so the user can fix their graph and try again. A multi-year graph can easily hold dozens of such problems, so the user needs to be able to work through them one at a time.

Reproduce the Bug

Each example below is a tiny synthetic graph (config: {:meta/version 1 :journal/page-title-format "yyyy-MM-dd EEEE"}). I reproduced all of them on upstream/master at 0232648eb4 with the headless importer. None contains real data.

# Input What the importer says today What is actually wrong
1 A Markdown file at the graph root (not under pages/ or journals/) containing finish:: [[2024-07-18 Thursday]], plus journals/2024_07_18.md. Root files are imported before journals. Nothing found for entity id [:block/uuid #uuid "00000001-2024-0718-0000-000000000000"] A property links a journal page that does not exist yet when the root file is imported. The same file under pages/ imports fine. OG never read root-level files, so these are usually stray copies.
2 journals/2022_03_27.md: - {{query (and (todo now doing))}} with query-sort-by:: clock-time No ident found for :clock-time OG's query tables had a built-in clock-time column that the importer does not recognise.
3 journals/2022_04_19.md: an advanced query with an unclosed bracket: - #+BEGIN_QUERY / :query [:find (pull ?h [*]) / #+END_QUERY :query is not ISeqable The query text is malformed (unbalanced brackets).
4 pages/ %2ARemembrance%2A.md, with a leading space and %2A in the file name No uuid found for page name " *remembrance*" The page name decoded from the file name starts with a space and contains markdown emphasis characters.
5 pages/T.md whose first line is a page-level property template:: x (same with template-including-parent:: false) No property schema found for nil Page-level template:: properties are unsupported. The same property on a block imports fine.
6 pages/A.md with vegetarian:: false and pages/B.md with vegetarian:: yes Expected number or lookup ref for entity id, got nil One property is a boolean on one page and yes on another, so the value cannot be stored.
7 pages/a.md: {{query (property type book)}} with query-sort-by:: title, and pages/z.md with title:: Dune No ident found for :title title:: is a real property, but it is looked up while importing a.md, before any page that defines it has been processed. The result depends on file order.
8 pages/People.md with a block property discord:: skylyn, and a namespaced page pages/Logseq%2FDiscord.md (title Logseq/Discord, so its leaf is Discord) Headless importer: completes, then validation reports :user.property/discord with should be a valid :db/ident namespace and blocks with invalid dispatch value. Desktop app: the scan's write is rejected as DB write failed with invalid data, and the toast says only Scanning the folder failed. A property and the leaf of a namespace page share the name "discord", so the property cannot get its own name. Renaming the property (for example discord-handle::) imports cleanly.

logseq-import-1486-repros.zip

Steps in the app: choose Import, then File to DB graph, pick the folder, and submit. With any of the inputs above you see Unexpected error: Scanning the folder failed, followed by the worker restart described above.

For a sense of scale, one real graph (about 1,400 journals and 450 pages) hit all eight of these, plus other variations, and I found them one at a time.

Expected Behavior

Fail fast, say exactly why, and make it easy to retry.

  1. Stop at the first failure. Do not carry on into a half-built graph.
  2. Name the problem in terms the user can act on. At minimum the file path relative to the graph folder. The block or line when known. Then the category and a plain-language reason with a suggested fix. For example, for case 7: pages/a.md: the query sorts by "title", which is not known yet at this point of the import. Remove the query-sort-by line or move it below a page that defines "title". Keep the raw error and ex-data in the log for developers, not as the user message.
  3. Make restarting cheap. After the user edits the file and starts the import again, they should be able to continue working through the list. A cancelled or failed import should leave no empty or partial graph behind (see #1469).
  4. Never take down the worker. A rejected write during the dry-run scan should come back to the renderer as an error carrying its message and the file, not as a dropped connection.
  5. Show the cause, not just the wrapper. The toast, or better a dialog in the existing import report style, should show the underlying message. The wrapper text can stay as a heading.

Not asked for: making the importer accept any of the inputs above, or reporting all failures in one run (although that would be a good later improvement for very large graphs).

Screenshots

I can attach the generic toast and the console output showing ERR_EMPTY_RESPONSE followed by the worker restart if useful.

Desktop or Mobile Platform Information

macOS (Apple silicon), Logseq desktop app built from upstream/master at 0232648eb4, run with pnpm electron-watch. Headless reproduction with bb dev:import on the same commit.

Additional context

  • logseq.db/transact-sync logs :transact-failed with (str error), which is why the worker log only shows [object Object]. The same code returns the real error in the headless path.
  • imports.cljs abort-file-graph-import! builds the notification from (.-message error), and the scan wrapper's cause is not read.
  • Minimal graphs for all eight cases are in https://github.com/jpjpjp/logseq-og-clock-import-fixture/tree/main/import-failures-1486, one folder per row of the table above, with a README listing the exact message each produces. They fail the same way on upstream/master at 0232648eb4.
Lenguaje dominante
Sin datos de lenguaje
Estrellas
29
Forks
2
Sin PR fusionados en 30 d

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de logseq/db-test

Todos los issues de logseq/db-test

Issues similares

Más issues de Backend & API Design

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.