🥅 Add parser state and #detailed_message to ResponseParseError#599
Merged
🥅 Add parser state and #detailed_message to ResponseParseError#599
#detailed_message to ResponseParseError#599Conversation
59b9871 to
72f0023
Compare
Adding this parser state ivars onto the parse error will aid in debugging, e.g to print the parser's debug output in a different context.
This allows `ResponseParseError#detailed_message` to be used to create the parse error debug output _outside_ of the parser. When `Net::IMAP.debug` is true, `ResponseParseError#detailed_message` automatically includes the `parser_state` debug info. `ResponseParser` explicitly sets `parser_state` when printing the detailed message, because its own `config.debug?` is true.
Now _all_ of the parse_error debug message is available from `ResponseParseError#detailed_message`. This also makes a subtle change to the original: filtering is now done based on `parser_class`, rather than on file name. This should allow the debug messages to work correctly in custom parser subclasses.
72f0023 to
d13d88e
Compare
nevans
added a commit
that referenced
this pull request
Feb 10, 2026
`resp-text` is defined as: ```abnf resp-text = ["[" resp-text-code "]" SP] text ; RFC3501 resp-text = ["[" resp-text-code "]" SP] [text] ; RFC9051 ``` And our even more lenient interpretation (which incorporates RFC9051 errata), is this: ```abnf resp-text = ["[" resp-text-code "]" [SP [text]] / [text] ``` And, although the `resp-text-code` grammar is elaborate, the final alternative is a superset of every other alternative (in both RFCs): ```abnf resp-text-code /= atom [SP 1*<any TEXT-CHAR except "]">] ``` Notice that `text` is a superset of `atom`, `SP`, `"["`, `"]"`, and every other allowed character in `resp-text-code`. So, even if `resp-text-code` fails, `resp-text` may still succeed by parsing everything as `text`. However, (prior to this commit) the parser commits to `resp-text-code` as soon as `"["` is encountered at the beginning of `resp-text`. If what follows is not `resp-text-code`, maybe it doesn't begin with an `atom` or it doesn't have a closing `"]"`, then we fail to parse `resp-text` correctly. Fixing this requires either looking further ahead more than a single token or backtracking when `resp-text-code` fails to parse. In this case, I think backtracking is the best approach. We don't need to worry about backtracking taking exponential time, because the fallback (`text`) is exceptionally simple (does not call any other productions), will parse all the way up to the next CRLF, and so nothing backtrackable can be nested. This uses `ResponseParser#current_state` that was added for #599 and relies on #600 to ensure that debug error messages are not printed when backtracking. Fixes #597.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding the parser state ivars onto parse errors will aid in debugging, e.g allowing someone to inspect or print the parser's debug output in a different context (not merely as output to
$stderr, and only whenNet::IMAP.debug?is enabled).Adding the context to
ResponseParseErroralso allows us to move the parse error debug output into#detailed_message. This is a much more natural home for it. By default, it will only show theparser_statewhenNet::IMAP.debug?is true, and it won't show theparser_backtrace.ResponseParserexplicitly sets both totrue, when its ownconfig.debug?is true.This also modifies the original debug output in a few small ways: the first line uses
superto get the standardException#detailed_message, including highlighting, and backtrace filtering is now done based onparser_class(rather than on file path). In my local tests, that seems to work okay, both for subclasses and for methods inherited from modules.Additionally, I've added a failsafe, so an exception inside
#detailed_messageshould still return something useful!