Improving error messages

max-sixty commented 1 year ago

What's up?

Some of our error messages could be much better. We now have the Chumsky parser, on top of the ASTs, which should allow us to make these much great. At the moment, they're anywhere from "much better than SQL" to "a bit worse"...

Here's an example of a really bad error message:

from film
group

Error:
    ╭─[:?:?]
    │
    │
    │
    │ Help: are you missing `from` statement?
 ───╯

Probably one-issue-per-error-message is not a good way of managing them — there would be a huge number of issues, and it would be difficult to see what's current. So instead, I created a file test_bad_error_messages where we can record bad error messages and seeded it with the above.

Two forms of contributions would be very helpful:

Add bad error messages
Take a bad error message, make it good, and move the test to test_error_messages. The messages there are kept up-to-date with tests on every commit, so there's no risk of wasted effort, and the change can't regress. I left some quick notes for how to approach this, and it's also very encouraged to ask questions.
- Link back to this issue in the PR, so other folks can we can gradually get a few examples of how these are done

khoa165 commented 1 year ago

Hi @max-sixty, I am pretty new to Open source in general so I would love to give this a try as it seems doable. Do I just make a PR that references this issue?

max-sixty commented 1 year ago

Hi @khoa165 — very much so re opening a PR.

There are more details on our contribution process here.

How experienced with Rust are you? Some of the error messages will be easy-moderate difficulty. Some will be quite hard! And I don't have a confident view on which are which. So feel free to ask for help, to post something half-formed which doesn't yet work, etc.

If you're less experienced, I'd also encourage starting with any small contribution; I've found that a great way to work up to bigger contributions.

khoa165 commented 1 year ago

Hi @max-sixty, definitely first time seeing in Rust syntax so a bit I'm definitely "rusty" on this 😅 I worked with some other languages so hopefully I can pick up Rust as I start poking around and contributing more.

khoa165 commented 1 year ago

I have a question. Your existing function array_instead_of_tuple seems to indicate that the following snippet would result in error

from e=employees
    select [e.first_name, e.last_name]

However, I was pasting it on playground and did not see any error message from it. It seems to be a valid query. Screenshot 2023-06-20 at 12 04 38 AM

Also, if I have future question, do I comment here? Or is there a faster way to reach you/other core maintainers?

khoa165 commented 1 year ago

Similar case/question, I wonder why the expected error message in the unit test is different from the playground for this case as well

Screenshot 2023-06-20 at 12 09 53 AM

eitsupi commented 1 year ago

@khoa165 Hi, thank you for looking at that.

Playground is based on the last release (currently 0.8.1) and is out of date compared to the latest development version (the main branch).

max-sixty commented 1 year ago

task run-playground will let you run a local version of the code, or you can run echo "from tracks" | cargo run -p prqlc -- compile...

khoa165 commented 1 year ago

@max-sixty I wonder if you might know why this error happens For context, earlier today I ran task setup-dev and then just now, I ran task run-playground

Screenshot 2023-06-20 at 1 11 58 AM

max-sixty commented 1 year ago

I don't see the cause immediately. Is there anything above it? Can you paste the full log somewhere rather than a screenshot?

Did task setup-dev complete successfully?

khoa165 commented 1 year ago

@max-sixty Here is the full output

Failed to compile.

Module not found: Error: Can't resolve 'prql-js/dist/bundler' in '/Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/src/workbench'
assets by path static/js/*.js 14.2 MiB
  assets by chunk 513 KiB (id hint: vendors)
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_language_json_jsonMode_js.chunk.js 86.5 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_language_html_htmlMode_js.chunk.js 72.3 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_language_css_cssMode_js.chunk.js 71.2 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_language_typescript_tsMode_js.chunk.js 44.7 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_basic-languages_freemarker2_freemarker2_js.chunk.js 26.6 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_basic-languages_solidity_solidity_js.chunk.js 21.1 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_basic-languages_powerquery_powerquery_js.chunk.js 19.1 KiB [emitted] (id hint: vendors) 1 related asset
    asset static/js/vendors-node_modules_monaco-editor_esm_vs_basic-languages_abap_abap_js.chunk.js 16.7 KiB [emitted] (id hint: vendors) 1 related asset
    + 12 assets
  + 65 assets
asset static/media/codicon.4a477f66bfd4e4d3cf68.ttf 71.7 KiB [emitted] [immutable] [from: node_modules/monaco-editor/esm/vs/base/browser/ui/codicons/codicon/codicon.ttf] (auxiliary name: main)
asset asset-manifest.json 33.9 KiB [emitted]
asset index.html 1.77 KiB [emitted]
1927 modules

ERROR in ./src/workbench/Workbench.js 5:0-45
Module not found: Error: Can't resolve 'prql-js/dist/bundler' in '/Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/src/workbench'
resolve 'prql-js/dist/bundler' in '/Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/src/workbench'
  Parsed request is a module
  using description file: /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/package.json (relative path: ./src/workbench)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/src/workbench/node_modules doesn't exist or is not a directory
      /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/src/node_modules doesn't exist or is not a directory
      looking for modules in /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules
        existing directory /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js
          using description file: /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/package.json (relative path: .)
            using description file: /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/package.json (relative path: ./dist/bundler)
              no extension
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler doesn't exist
              .web.mjs
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.web.mjs doesn't exist
              .mjs
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.mjs doesn't exist
              .web.js
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.web.js doesn't exist
              .js
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.js doesn't exist
              .json
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.json doesn't exist
              .web.jsx
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.web.jsx doesn't exist
              .jsx
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.jsx doesn't exist
              as directory
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler doesn't exist
      /Users/khoa165/Desktop/code/projects/opensource/prql/web/node_modules doesn't exist or is not a directory
      looking for modules in /Users/khoa165/Desktop/code/projects/opensource/prql/node_modules
        /Users/khoa165/Desktop/code/projects/opensource/prql/node_modules/prql-js doesn't exist
      /Users/khoa165/Desktop/code/projects/opensource/node_modules doesn't exist or is not a directory
      /Users/khoa165/Desktop/code/projects/node_modules doesn't exist or is not a directory
      /Users/khoa165/Desktop/code/node_modules doesn't exist or is not a directory
      /Users/khoa165/Desktop/node_modules doesn't exist or is not a directory
      /Users/khoa165/node_modules doesn't exist or is not a directory
      /Users/node_modules doesn't exist or is not a directory
      /node_modules doesn't exist or is not a directory
      looking for modules in /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules
        existing directory /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js
          using description file: /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/package.json (relative path: .)
            using description file: /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/package.json (relative path: ./dist/bundler)
              no extension
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler doesn't exist
              .web.mjs
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.web.mjs doesn't exist
              .mjs
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.mjs doesn't exist
              .web.js
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.web.js doesn't exist
              .js
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.js doesn't exist
              .json
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.json doesn't exist
              .web.jsx
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.web.jsx doesn't exist
              .jsx
                Field 'browser' doesn't contain a valid alias configuration
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler.jsx doesn't exist
              as directory
                /Users/khoa165/Desktop/code/projects/opensource/prql/web/playground/node_modules/prql-js/dist/bundler doesn't exist
 @ ./src/app/App.js 5:0-47 72:37-46
 @ ./src/index.js 8:0-28 13:33-36

webpack 5.76.1 compiled with 1 error in 37288 ms

I think task setup-dev was success. I just ran that again and still see green (output below)

task: [install-cargo-tools] 
  task install-cargo-tools-source

task: [install-cargo-tools-source] cargo install --locked bacon cargo-audit cargo-insta cargo-release default-target mdbook mdbook-admonish mdbook-footnote mdbook-toc wasm-bindgen-cli wasm-opt wasm-pack
    Updating crates.io index
     Ignored package `bacon v2.9.0` is already installed, use --force to override
     Ignored package `cargo-audit v0.17.6` is already installed, use --force to override
     Ignored package `cargo-insta v1.29.0` is already installed, use --force to override
     Ignored package `cargo-release v0.24.10` is already installed, use --force to override
     Ignored package `default-target v0.1.0` is already installed, use --force to override
     Ignored package `mdbook v0.4.30` is already installed, use --force to override
     Ignored package `mdbook-admonish v1.9.0` is already installed, use --force to override
     Ignored package `mdbook-footnote v0.1.1` is already installed, use --force to override
     Ignored package `mdbook-toc v0.12.0` is already installed, use --force to override
     Ignored package `wasm-bindgen-cli v0.2.87` is already installed, use --force to override
     Ignored package `wasm-opt v0.112.0` is already installed, use --force to override
     Ignored package `wasm-pack v0.12.0` is already installed, use --force to override
     Summary Successfully installed bacon, cargo-audit, cargo-insta, cargo-release, default-target, mdbook, mdbook-admonish, mdbook-footnote, mdbook-toc, wasm-bindgen-cli, wasm-opt, wasm-pack!
task: Task "install-brew-dependencies" is up to date
task: [install-maturin] python3 -m pip install -U maturin
Requirement already satisfied: maturin in /usr/local/lib/python3.11/site-packages (1.1.0)

[notice] A new release of pip is available: 23.0.1 -> 23.1.2
[notice] To update, run: python3.11 -m pip install --upgrade pip
task: Task "install-pre-commit" is up to date
task: [install-npm-dependencies] npm install --location=global prettier prettier-plugin-go-template
npm WARN saveError ENOENT: no such file or directory, open '/Users/khoa165/Desktop/code/projects/opensource/prql/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/Users/khoa165/Desktop/code/projects/opensource/prql/package.json'
npm WARN prql No description
npm WARN prql No repository field.
npm WARN prql No README data
npm WARN prql No license field.

+ prettier-plugin-go-template@0.0.13
+ prettier@2.8.8
updated 2 packages and audited 4 packages in 0.893s

1 package is looking for funding
  run `npm fund` for details

found 0 vulnerabilities

In order to get nice auto-formatting of web code in VS Code, VS Code requires configuration to use the system-wide install of prettier. See https://github.com/NiklasPor/prettier-plugin-go-template/issues/58#issuecomment-1085060511 for more info.

🟡 It looks like VS Code is installed but doesn't have all recommended extensions installed:
❌ budparr.language-hugo-vscode 
✅ esbenp.prettier-vscode 
❌ mitsuhiko.insta 
✅ prql-lang.prql-vscode 
✅ rust-lang.rust-analyzer 

Install them with:

  task install-vscode-extensions

task: [setup-dev] pre-commit install-hooks

🟢 Setup complete! ✅🚀

max-sixty commented 1 year ago

To confirm, the initial log is from running task run-playground?

It looks like something might be up with npm there:

npm WARN saveError ENOENT: no such file or directory, open '/Users/khoa165/Desktop/code/projects/opensource/prql/package.json'

I'm no npm expert unfortunately. What do you get with npm --version? I get 9.6.7

khoa165 commented 1 year ago

@max-sixty yes, the initial log is from running task run-playground. Hmmm, my npm version is only 6.14.14 I see in node modules prql-js does not have dist folder so maybe that has something to do Screenshot 2023-06-20 at 9 54 54 AM

Also now that i look at task setup-dev log above carefully, i found this suspicious part. It seems like it tries to find package.json at the root of project but can't find. I check manually in the codebase and don't see it at the root project directory either

[notice] A new release of pip is available: 23.0.1 -> 23.1.2
[notice] To update, run: python3.11 -m pip install --upgrade pip
task: Task "install-pre-commit" is up to date
task: [install-npm-dependencies] npm install --location=global prettier prettier-plugin-go-template
npm WARN saveError ENOENT: no such file or directory, open '/Users/khoa165/Desktop/code/projects/opensource/prql/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN enoent ENOENT: no such file or directory, open '/Users/khoa165/Desktop/code/projects/opensource/prql/package.json'
npm WARN prql No description
npm WARN prql No repository field.
npm WARN prql No README data
npm WARN prql No license field.

eitsupi commented 1 year ago

@max-sixty @khoa165 Issues related to setting up the development environment seem to be off topic. I suggest you open a new issue or discussion.

eitsupi commented 1 year ago

@khoa165 Looking at the Taskfile, it seems that the npm installation is not managed on the Taskfile, try updating npm. It is a command to install packages globally, so there should be no need to refer to package.json.

max-sixty commented 1 year ago

@max-sixty @khoa165 Issues related to setting up the development environment seem to be off topic. I suggest you open a new issue or discussion.

Yes, sorry, my fault. I was planning to minimize the comments like you have. But I should have opened a new issue anyway.

PRQL / prql

Improving error messages #2282

What's up?