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

Send runtime error to the frontend #359

Merged
merged 3 commits into from
May 16, 2024
Merged

Send runtime error to the frontend #359

merged 3 commits into from
May 16, 2024

Conversation

greg-finley
Copy link
Contributor

Closes #358

Open to other ideas as well

Screenshot 2024-05-15 at 1 11 23 PM

@greg-finley greg-finley requested a review from nolanbconaway May 15, 2024 20:16
Copy link
Member

@nolanbconaway nolanbconaway left a comment

Choose a reason for hiding this comment

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

I think this approach makes sense! But is RuntimeError the correct level of specificity?

I wonder if the app should have a handler for generic errors that might not be Runtime; in which case maybe the better approach could look like a broad error handler which returns an error page that contains exception info and maybe a link to the sqlfluff issues page.

That second option seems like more code to write, so in the meantime i think we should merge in your changes. I can set up an issue and work on it this weekend if you think the error page idea makes sense.

@greg-finley
Copy link
Contributor Author

It's up to you! I just reached for this solution to give you similar text to what the CLI would give you, which has actionable steps.

I'm actually not sure the enumeration of possible errors beyond RuntimeError but it's only one I've seen raised by lint.

I'm happy to live with this PR or let you do a different solution.

@nolanbconaway
Copy link
Member

Ok! Lets get this in and i'll toy with an error page later on.

@nolanbconaway
Copy link
Member

#360

@nolanbconaway nolanbconaway merged commit cf71e6d into master May 16, 2024
1 check passed
@nolanbconaway nolanbconaway deleted the runtime-error branch May 16, 2024 12:58
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.

More gracefully handle errors
2 participants