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

Handle internal errors elegantly and report the triggering sql file #632

Merged
merged 7 commits into from
Dec 14, 2020

Conversation

NiallRees
Copy link
Member

@NiallRees NiallRees commented Dec 11, 2020

At present, if an attempt to lint a file fails due to an unhandled internal error, the linting process quits and doesn't report which file is causing the issue. This makes it hard to identify what file has surfaced the internal error, which you need to do in order to .sqlfluffignore the file and work around it. This PR prints the exception as a warning with the file that has surfaced the issue.

Outcome:

WARNING    Unable to lint /Users/niallwoodward/dev/.../models/reporting/query.sql due to an internal error. Please report this as an issue with your query's contents and stacktrace below!
To hide this warning, add the failing file to .sqlfluffignore
Traceback (most recent call last):
  File "/Users/niallwoodward/dev/pull_requests/sqlfluff/src/sqlfluff/core/linter.py", line 1254, in lint_path
    target_file.read(), fname=fname, fix=fix, config=config
  File "/Users/niallwoodward/dev/pull_requests/sqlfluff/src/sqlfluff/core/linter.py", line 1044, in lint_string
    parsed = self.parse_string(in_str=in_str, fname=fname, config=config)
  File "/Users/niallwoodward/dev/pull_requests/sqlfluff/src/sqlfluff/core/linter.py", line 816, in parse_string
    tokens, lex_vs = lexer.lex(templated_file)
  File "/Users/niallwoodward/dev/pull_requests/sqlfluff/src/sqlfluff/core/parser/lexer.py", line 343, in lex
    return self.enrich_segments(segment_buff, raw), violations
  File "/Users/niallwoodward/dev/pull_requests/sqlfluff/src/sqlfluff/core/parser/lexer.py", line 373, in enrich_segments
    templated_slice
  File "/Users/niallwoodward/dev/pull_requests/sqlfluff/src/sqlfluff/core/templaters/base.py", line 289, in templated_slice_to_source_slice
    if stop_slices[-1][0] == "literal":
IndexError: list index out of range

WARNING    Indent balance test failed for '/Users/niallwoodward/dev/.../models/reporting/query2.sql'. Template indents will not be linted for this file.
WARNING    Indent balance test failed for '/Users/niallwoodward/dev/.../models/reporting/query3.sql'. Template indents will not be linted for this file.
==== summary ====
violations:        0 status:         PASS
______________________________________________________________________________________________________________ summary _______________________________________________________________________________________________________________
  sqlfluff: commands succeeded
  congratulations :)

@NiallRees NiallRees requested a review from a team December 11, 2020 15:16
@NiallRees NiallRees force-pushed the handle_internal_errors_elegantly branch 2 times, most recently from 2449b0b to 7863508 Compare December 11, 2020 15:24
@NiallRees NiallRees force-pushed the handle_internal_errors_elegantly branch from 7863508 to 6e6929f Compare December 11, 2020 15:52
@NiallRees NiallRees force-pushed the handle_internal_errors_elegantly branch from f1418cb to 809fc66 Compare December 12, 2020 17:12
Copy link
Member

@pwildenhain pwildenhain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never had seen the traceback module before, cool stuff 😎 .

Would you be able to at a test cast for a file that will cause an internal error, and make sure it gets the nice error message?

@NiallRees
Copy link
Member Author

Never had seen the traceback module before, cool stuff 😎 .

Would you be able to at a test cast for a file that will cause an internal error, and make sure it gets the nice error message?

Had a lot of fun working out how to write a test case for this. Good call and thanks!

@dmateusp
Copy link
Contributor

@NiallRees I'm not sure exactly how to test that PR haha, but just wanted to double check that we know which file in a directory caused the internal issue in case it's raised?

I'm seeing your test is on a specific file like sqlfluff lint my_project/b/x.sql and not sqlfluff lint my_project/ for example

@NiallRees
Copy link
Member Author

@NiallRees I'm not sure exactly how to test that PR haha, but just wanted to double check that we know which file in a directory caused the internal issue in case it's raised?

I'm seeing your test is on a specific file like sqlfluff lint my_project/b/x.sql and not sqlfluff lint my_project/ for example

I did this as a follow up to #600, where it took ages to work out what sql file was surfacing the error. Does that answer your question? Not sure I quite understand.

Copy link
Member

@pwildenhain pwildenhain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll sleep better knowing this test is here 😴

@dmateusp
Copy link
Contributor

Ah, I got confused, thanks for linking the issue

@NiallRees NiallRees marked this pull request as draft December 14, 2020 10:04
@NiallRees NiallRees marked this pull request as ready for review December 14, 2020 11:26
@NiallRees NiallRees merged commit c8ee3de into master Dec 14, 2020
@NiallRees NiallRees deleted the handle_internal_errors_elegantly branch December 14, 2020 11:26
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

Successfully merging this pull request may close these issues.

3 participants