Skip to content

OpenAI: log errors#614

Merged
glebm merged 4 commits intoglebm:mainfrom
navidemad:feat-openai-logging
Feb 9, 2025
Merged

OpenAI: log errors#614
glebm merged 4 commits intoglebm:mainfrom
navidemad:feat-openai-logging

Conversation

@navidemad
Copy link
Copy Markdown
Contributor

@navidemad navidemad commented Jan 25, 2025

bundle exec i18n-tasks translate-missing --backend=openai 
bundler: failed to load command: i18n-tasks (/Users/dev/.rbenv/versions/3.4.1/lib/ruby/gems/3.4.0/bin/i18n-tasks)                                                                                                                           >  ETA: ??:??:?? 0/161 (0%)
/Users/dev/.rbenv/versions/3.4.1/lib/ruby/gems/3.4.0/gems/faraday-2.12.2/lib/faraday/response/raise_error.rb:30:in 'Faraday::Response::RaiseError#on_complete': the server responded with status 400 (Faraday::BadRequestError)

In order to have more detailed informations you can pass now OPENAI_LOG_ERRORS as environment variable to display more informations.

@dankimio
Copy link
Copy Markdown
Contributor

dankimio commented Feb 3, 2025

Removing f.response :raise_error in ruby-openai also helps — openai/http.rb#L79

connection = Faraday.new do |f|
  f.options[:timeout] = @request_timeout
  f.request(:multipart) if multipart
  f.use MiddlewareErrors if @log_errors
  # f.response :raise_error
  f.response :json
end

Not sure why they chose to hide log output from errors.

@translator ||=
if @i18n_tasks.translation_config[:openai_log_errors]
OpenAI::Client.new(access_token: api_key, log_errors: @i18n_tasks.translation_config[:openai_log_errors]) do |f|
f.response :logger, Logger.new($stdout), bodies: true
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

  1. Errors should be logged to STDERR
  2. I think we can always log the errors, no need for the environment variable

@navidemad
Copy link
Copy Markdown
Contributor Author

Differents repository are recommanding to set this option only to true for development, as it can leak leak private data to your logs.
So i made it related to RAILS_ENV / RAKE_ENV.

access_token: api_key,
# Highly recommended in development, so you can see what errors OpenAI is returning.
# Not recommended in production because it could leak private data to your logs.
log_errors: ENV.fetch('RAILS_ENV', ENV.fetch('RACK_ENV', '')) == 'development'
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

RAILS_ENV and RACK_ENV are usually not defined when running i18n-tasks.
While it supports Rails, it does it entirely via static analysis, and does not load rails itself.

It is possible of course to load i18n-tasks as a library from within rails, in which case it'd be good to respect this.

However, most of the time, this string will be empty. So let's also log errors when it's empty.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok i see. Anyways, in your README.md most of the time the gem will be installed only in the group :development anyways.

@glebm glebm changed the title Implement ENV["OPENAI_LOG_ERRORS"] to display OpenAI http log_errors OpenAI: log errors Feb 9, 2025
@glebm glebm merged commit c66cd86 into glebm:main Feb 9, 2025
bart-westenenk-bex pushed a commit to bookingexperts/i18n-tasks that referenced this pull request Dec 10, 2025
bart-westenenk-bex pushed a commit to bookingexperts/i18n-tasks that referenced this pull request Dec 10, 2025
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