Skip to content

Review #2082 for code improvements: each file source handles its own errors #2677

Open

Description

@mathiasrw

This is not a bug report. It is an invitation to look at #1919 and #2082 again, because each file source handles its own errors, and the copies differ. The goal is to understand the change and decide whether anything should be improved. The notes below come from a quick read of the diffs and a few runs, not from a full analysis.

The concerns:

  • The JSON error handler is never called. In alasql.from.JSON (src/84from.js) the handler Fix JSONL source JOIN & Improve error handling for JSON & JSONL source #2082 improved sits after loadFile(...) behind a comma, (loadFile(...), err => {...}), so it is not passed to loadFile. A missing file ends in an uncaught error is not a function, and alasql.promise never settles. Fix JSON file error handling #2513 fixes this.
  • A parse error is thrown inside the load callback, for JSON and JSONL alike. With alasql.promise, SELECT * FROM JSONL('bad.jsonl') throws an uncaught Could not parse JSON at line 1 and the promise never settles. Fix JSON file error handling #2513 covers JSON only.
  • Routing both read errors and parse errors to the query's callback in one place (in loadFile, or one wrapper all file sources use) would fix every source at once. I have not checked the other sources (TXT, CSV, XLSX, …).

Related: #1314, #2513.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions