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

Refactor user errors in lexer from panics to error return values #3428

Closed
turbolent opened this issue Jun 18, 2024 · 2 comments · Fixed by #3578
Closed

Refactor user errors in lexer from panics to error return values #3428

turbolent opened this issue Jun 18, 2024 · 2 comments · Fixed by #3578
Assignees
Labels
Bug Something isn't working Feedback Technical Debt

Comments

@turbolent
Copy link
Member

Issue to be solved

The lexer currently reports user errors through panics (In particular, there is currently only one user error, "token limit reached", i.e. TokenLimitReachedError).

As the lexer is used in the parser, which already previously got refactored from reporting user errors as panics in #1764, it might be surprising to users of the parser that user errors are still reported as panics in some cases.

Suggested Solution

Refactor the lexer to report user errors, such as the "token limit reached" error (TokenLimitReachedError), to be reported as an error return value, instead of a panic.

@turbolent turbolent added Feature Feedback Bug Something isn't working Technical Debt and removed Feature labels Jun 18, 2024
@turbolent
Copy link
Member Author

In addition to the token limit reached error, there's also the memory usage error (errors.MemoryError) which is a user error, but a fatal error (stop parsing).

@SupunS
Copy link
Member

SupunS commented Aug 1, 2024

Memory error is currently a panic not only in the lexer, but also in parser. We would need to refactor both the lexer and parser parser to return memory error, instead of panicking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Feedback Technical Debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants