fix: log only on system api key failures (#13968)
Removes sentry flooding of unnecessary rubyllm logs of wrong API key. Logs only system api key error since it would be P0. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -27,9 +27,9 @@ class Captain::Llm::TranslateQueryService < Captain::BaseTaskService
|
||||
end
|
||||
|
||||
# Translation is an internal operation, not customer-initiated.
|
||||
# Prefer the system key; fall back to the account's hook key for self-hosted setups without one.
|
||||
def api_key
|
||||
@api_key ||= system_api_key.presence || openai_hook&.settings&.dig('api_key')
|
||||
# It should always use the installation key.
|
||||
def llm_credential
|
||||
@llm_credential ||= system_llm_credential
|
||||
end
|
||||
|
||||
def query_in_target_language?(query)
|
||||
|
||||
@@ -56,12 +56,10 @@ class Captain::ConversationCompletionService < Captain::BaseTaskService
|
||||
{ complete: false, reason: reason }
|
||||
end
|
||||
|
||||
# Prefer the system API key over the account's OpenAI hook key.
|
||||
# This is an internal operational evaluation, not a customer-triggered feature,
|
||||
# so it should not consume the customer's OpenAI credits on hosted platforms.
|
||||
# Falls back to the account hook for self-hosted deployments without a system key.
|
||||
def api_key
|
||||
@api_key ||= system_api_key.presence || openai_hook&.settings&.dig('api_key')
|
||||
# so it should always use the installation key.
|
||||
def llm_credential
|
||||
@llm_credential ||= system_llm_credential
|
||||
end
|
||||
|
||||
def event_name
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
class Captain::BaseTaskService
|
||||
include Integrations::LlmInstrumentation
|
||||
include Captain::ToolInstrumentation
|
||||
include Llm::ExceptionTrackable
|
||||
|
||||
# gpt-4o-mini supports 128,000 tokens
|
||||
# 1 token is approx 4 characters
|
||||
@@ -55,7 +56,9 @@ class Captain::BaseTaskService
|
||||
end
|
||||
|
||||
def execute_ruby_llm_request(model:, messages:, schema: nil, tools: [])
|
||||
Llm::Config.with_api_key(api_key, api_base: api_base) do |context|
|
||||
credential = llm_credential
|
||||
|
||||
Llm::Config.with_api_key(credential[:api_key], api_base: api_base) do |context|
|
||||
chat = build_chat(context, model: model, messages: messages, schema: schema, tools: tools)
|
||||
|
||||
conversation_messages = messages.reject { |m| m[:role] == 'system' }
|
||||
@@ -65,7 +68,7 @@ class Captain::BaseTaskService
|
||||
build_ruby_llm_response(chat.ask(conversation_messages.last[:content]), messages)
|
||||
end
|
||||
rescue StandardError => e
|
||||
ChatwootExceptionTracker.new(e, account: account).capture_exception
|
||||
capture_llm_exception(e, credential: credential)
|
||||
{ error: e.message, request_messages: messages }
|
||||
end
|
||||
|
||||
@@ -147,11 +150,24 @@ class Captain::BaseTaskService
|
||||
end
|
||||
|
||||
def api_key_configured?
|
||||
api_key.present?
|
||||
llm_credential.present?
|
||||
end
|
||||
|
||||
def api_key
|
||||
@api_key ||= openai_hook&.settings&.dig('api_key') || system_api_key
|
||||
llm_credential&.dig(:api_key)
|
||||
end
|
||||
|
||||
def llm_credential
|
||||
@llm_credential ||= hook_llm_credential || system_llm_credential
|
||||
end
|
||||
|
||||
def hook_llm_credential
|
||||
key = openai_hook&.settings&.dig('api_key').presence
|
||||
{ api_key: key, source: :hook } if key
|
||||
end
|
||||
|
||||
def system_llm_credential
|
||||
{ api_key: system_api_key, source: :system } if system_api_key.present?
|
||||
end
|
||||
|
||||
def openai_hook
|
||||
@@ -162,6 +178,10 @@ class Captain::BaseTaskService
|
||||
@system_api_key ||= InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_API_KEY')&.value
|
||||
end
|
||||
|
||||
def exception_tracking_account
|
||||
account
|
||||
end
|
||||
|
||||
def prompt_from_file(file_name)
|
||||
Rails.root.join('lib/integrations/openai/openai_prompts', "#{file_name}.liquid").read
|
||||
end
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
class Integrations::LlmBaseService
|
||||
include Integrations::LlmInstrumentation
|
||||
include Llm::ExceptionTrackable
|
||||
|
||||
# gpt-4o-mini supports 128,000 tokens
|
||||
# 1 token is approx 4 characters
|
||||
@@ -100,13 +101,14 @@ class Integrations::LlmBaseService
|
||||
def execute_ruby_llm_request(parsed_body)
|
||||
messages = parsed_body['messages']
|
||||
model = parsed_body['model']
|
||||
credential = llm_credential
|
||||
|
||||
Llm::Config.with_api_key(hook.settings['api_key'], api_base: api_base) do |context|
|
||||
Llm::Config.with_api_key(credential[:api_key], api_base: api_base) do |context|
|
||||
chat = context.chat(model: model)
|
||||
setup_chat_with_messages(chat, messages)
|
||||
end
|
||||
rescue StandardError => e
|
||||
ChatwootExceptionTracker.new(e, account: hook.account).capture_exception
|
||||
capture_llm_exception(e, credential: credential)
|
||||
build_error_response_from_exception(e, messages)
|
||||
end
|
||||
|
||||
@@ -164,6 +166,14 @@ class Integrations::LlmBaseService
|
||||
}
|
||||
end
|
||||
|
||||
def llm_credential
|
||||
@llm_credential ||= { api_key: hook.settings['api_key'], source: :hook }
|
||||
end
|
||||
|
||||
def exception_tracking_account
|
||||
hook.account
|
||||
end
|
||||
|
||||
def build_error_response_from_exception(error, messages)
|
||||
{ error: error.message, request_messages: messages }
|
||||
end
|
||||
|
||||
11
lib/llm/exception_trackable.rb
Normal file
11
lib/llm/exception_trackable.rb
Normal file
@@ -0,0 +1,11 @@
|
||||
module Llm::ExceptionTrackable
|
||||
private
|
||||
|
||||
def capture_llm_exception(error, credential:)
|
||||
if credential && credential[:source] == :system
|
||||
ChatwootExceptionTracker.new(error, account: exception_tracking_account).capture_exception
|
||||
else
|
||||
Rails.logger.error("[LLM] account=#{exception_tracking_account&.id} #{error.class}: #{error.message}")
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -141,15 +141,15 @@ RSpec.describe Captain::ConversationCompletionService do
|
||||
service.perform
|
||||
end
|
||||
|
||||
it 'falls back to the account hook key when no system key exists' do
|
||||
it 'does not fall back to the account hook key when no system key exists' do
|
||||
InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_API_KEY').update!(value: nil)
|
||||
|
||||
expect(Llm::Config).to receive(:with_api_key).with('customer-own-key', api_base: anything).and_yield(mock_context)
|
||||
allow(mock_chat).to receive(:ask).and_return(
|
||||
instance_double(RubyLLM::Message, content: { 'complete' => true, 'reason' => 'Done' }, input_tokens: 10, output_tokens: 5)
|
||||
)
|
||||
expect(Llm::Config).not_to receive(:with_api_key)
|
||||
|
||||
service.perform
|
||||
result = service.perform
|
||||
|
||||
expect(result[:complete]).to be false
|
||||
expect(result[:reason]).to eq(I18n.t('captain.api_key_missing'))
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -258,6 +258,18 @@ RSpec.describe Captain::BaseTaskService do
|
||||
expect(result[:error]).to eq('API Error')
|
||||
expect(result[:request_messages]).to eq(messages)
|
||||
end
|
||||
|
||||
it 'does not track exceptions for account hook failures' do
|
||||
create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'hook-key' })
|
||||
|
||||
expect(Llm::Config).to receive(:with_api_key).with('hook-key', api_base: anything).and_raise(error)
|
||||
expect(ChatwootExceptionTracker).not_to receive(:new)
|
||||
|
||||
result = service.send(:make_api_call, model: model, messages: messages)
|
||||
|
||||
expect(result[:error]).to eq('API Error')
|
||||
expect(result[:request_messages]).to eq(messages)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#api_key' do
|
||||
@@ -276,6 +288,16 @@ RSpec.describe Captain::BaseTaskService do
|
||||
expect(service.send(:api_key)).to eq('test-key')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when no API key is configured' do
|
||||
before do
|
||||
InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_API_KEY')&.destroy
|
||||
end
|
||||
|
||||
it 'returns nil' do
|
||||
expect(service.send(:api_key)).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#prompt_from_file' do
|
||||
|
||||
28
spec/lib/integrations/llm_base_service_spec.rb
Normal file
28
spec/lib/integrations/llm_base_service_spec.rb
Normal file
@@ -0,0 +1,28 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Integrations::LlmBaseService do
|
||||
let(:account) { create(:account) }
|
||||
let(:inbox) { create(:inbox, account: account) }
|
||||
let(:conversation) { create(:conversation, account: account, inbox: inbox) }
|
||||
let(:hook) { create(:integrations_hook, :openai, account: account, settings: { 'api_key' => 'hook-key' }) }
|
||||
let(:event) { { 'name' => 'summarize', 'data' => { 'conversation_display_id' => conversation.display_id } } }
|
||||
let(:service) { described_class.new(hook: hook, event: event) }
|
||||
let(:error) { StandardError.new('API Error') }
|
||||
let(:body) { { model: 'gpt-4', messages: [{ role: 'user', content: 'Hello' }] }.to_json }
|
||||
|
||||
describe '#make_api_call' do
|
||||
before do
|
||||
allow(service).to receive(:instrument_llm_call).and_yield
|
||||
allow(Llm::Config).to receive(:with_api_key).and_raise(error)
|
||||
end
|
||||
|
||||
it 'does not track exceptions for hook key failures' do
|
||||
expect(ChatwootExceptionTracker).not_to receive(:new)
|
||||
|
||||
result = service.send(:make_api_call, body)
|
||||
|
||||
expect(result[:error]).to eq('API Error')
|
||||
expect(result[:request_messages]).to eq([{ 'role' => 'user', 'content' => 'Hello' }])
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user