Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App crashes when data in localStorage references unexisting parser #684

Open
nene opened this issue Dec 14, 2022 · 0 comments
Open

App crashes when data in localStorage references unexisting parser #684

nene opened this issue Dec 14, 2022 · 0 comments

Comments

@nene
Copy link
Contributor

nene commented Dec 14, 2022

Describe the bug
When localStorage contains a reference to an unexisting parser, the whole AstExplorer app crashes.

To Reproduce
Steps to reproduce the behavior:

  1. Check out some PR that adds a new parser (e.g. Add new SQL parser: pgsql-ast-parser #682)
  2. Build and run AstExplorer locally
  3. Select the new parser in AstExplorer UI
  4. Switch back to master branch
  5. Rebuild and run AstExplorer
  6. Refresh the browser window
  7. Observe the page crashing

Expected behavior
No crash.

Screenshots
Screenshot 2022-12-14 at 19 00 16

Browser:

  • I experienced it in MacOS 12.6 Firefox 107.0.1, but I'm confident the problem is the same in all browsers.

astexplorer settings:

  • Selected parser: pgsql-ast-parser
  • Contents of the local storage key explorerSettingsV1: {"showTransformPanel":false,"parserSettings":{"sql-parser-cst":{"dialect":"bigquery","preserveComments":true,"includeRange":true}},"parserPerCategory":{"sql":"pgsql-ast-parser"},"workbench":{"parser":"pgsql-ast-parser","code":"SELECT * FROM tbl\n","keyMap":"default","transform":{"code":"","transformer":null}}}

The main culprit seems to be the value stored in workbench.parser field. When I change it to an existing parser, the app starts working.

Additional context

The invalid parser name can end up in localStorage in a few ways:

  1. When working on a branch that adds new parser to AstExplorer and then switching back to master branch.
  2. When the parser with that name used to be available in AstExplorer, but has been removed (or was renamed).

The first one is likely the most common scenario, and it's a very surprising and hard to debug behavior. One usually expects that switching back to the master will get one to a good state, but here the opposite happens - the feature branch is working, but somehow the master is broken. Cleaning node_modules and reinstalling all won't help you. Doing a whole clean checkout of the repository also won't fix it.

Today I spent a few hours trying to figure out what the heck is going wrong. I knew that master had been in working order before, but now it was broken, and I had no idea why. Even though I had actually experienced the exact same issue a few weeks ago and sorted it out, I had already forgotten about it and had to figure it all out again.

The second scenario is more of a possibility, as it seems that parsers mostly get added here. But I bet that at one day there will be a situation where a parser needs to be removed (e.g. it contains a serious vulnerability) or needs to be renamed. At which point this problem will come to bite regular users.

I found some other issues which hint at the same problem or at least something very similar:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant