-
-
Notifications
You must be signed in to change notification settings - Fork 241
Adding option for configuring custom log Regexp timeout #364
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean and simple change. I'll do the changes to the doc myself since I think we'd need a rewrite of the logging options in the configuration guide.
|
||
# Set custom timeout for log filtering | ||
# which is useful when working with large payloads | ||
config.log_regexp_timeout = 4 # seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not categorise this as basic logging. Let's remove that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be worth having a separate section called "Advanced logging options" where to put also the logger and log file options. Since that requires a rewrite of the section, I'm willing to take that on myself.
There's one problem with this: Regexp.timeout is only available from Ruby 3.1. Could you make it conditional to that having Ruby >= 3.2? |
Should we raise an error if a user sets |
I'd say output a warning and proceed |
What this does
Overview
This PR adds support for a configurable
log_regexp_timeout
used specifically byRubyLLM's
logging filters. It allows you to override the globalRegexp.timeout
for logging without affecting the rest of your application.Problem It Solves
When handling large or complex files, the default logging filters in
RubyLLM
may hit aRegexp::TimeoutError
. This is because many applications set a low globalRegexp.timeout
(e.g. 1 second) to protect against ReDoS, which can be too restrictive for logging large payloads.Solution
This change adds support for a
log_regexp_timeout
configuration value. If provided,RubyLLM
will use this timeout when compiling regular expressions for its Faraday logger filters. This isolates logging-related regex behavior from the broader application, improving reliability without compromising global safety.Example usage:
Type of change
Scope check
Quality check
overcommit --install
and all hooks passmodels.json
,aliases.json
)API changes
Related issues
Implements #331