From cdc5bd8a4f35d5fc5a128fe39eb4153ba75b5526 Mon Sep 17 00:00:00 2001 From: Milan Dufek Date: Wed, 29 Jul 2026 12:07:42 +0200 Subject: [PATCH 1/2] fix(lcm): bound error collections embedded in exception messages SynchronizeUserFilters (and the variables path) raised exceptions with the raw error Array/Hash as the message. Stringifying it runs Array#inspect / Hash#inspect over up to one entry per user/value of a whole domain sync, and every downstream #message / "#{e}" / JSON.pretty_generate call re-derives the same giant string (measured 53.9 MB for 200k entries, 4 call sites) -- exhausting the JRuby heap and firing LcmJavaHeapSpaceError (~50x/month on NA3). Cap the message (and the error log dump) to the first 10 entries plus a total count, following the existing synchronize_users.rb pattern. Full collections are no longer embedded in exceptions; bounded message is ~3 KB. JIRA: GRIF-951 Claude-Session: https://claude.ai/code/session_015xTbPyZxxECyx93VgEfW4p --- .../user_filters/user_filter_builder.rb | 27 ++++++++++++++++--- .../user_filters/user_filter_builder_spec.rb | 18 +++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/lib/gooddata/models/user_filters/user_filter_builder.rb b/lib/gooddata/models/user_filters/user_filter_builder.rb index ac139bb81..758742573 100644 --- a/lib/gooddata/models/user_filters/user_filter_builder.rb +++ b/lib/gooddata/models/user_filters/user_filter_builder.rb @@ -22,6 +22,11 @@ module UserFilterBuilder @all_domain_users = {} @mutex = Mutex.new + # Cap on error entries embedded in a raised exception's message (and logged). + # Domain-wide syncs can produce one entry per user/value across a whole domain; + # stringifying the raw collection exhausts the JRuby heap (GRIF-951). + MAX_ERRORS_IN_MESSAGE = 10 + # Main Entry function. Gets values and processes them to get filters # that are suitable for other function to process. # Values can be read from file or provided inline as an array. @@ -488,7 +493,9 @@ def self.execute_mufs(user_filters, options = {}) errors = errors.map do |e| e.merge(pid: project.pid) end - fail GoodData::FilterMaqlizationError, errors + sample = errors.take(MAX_ERRORS_IN_MESSAGE) + GoodData.logger.error("Maqlizing MUFs failed with #{errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") + fail GoodData::FilterMaqlizationError, "Maqlizing MUFs resulted in #{errors.size} error(s). First #{sample.size}: #{sample}" end filters = user_filters.map { |data| client.create(MandatoryUserFilter, data, project: project) } @@ -542,7 +549,11 @@ def self.execute_mufs(user_filters, options = {}) end project_log_formatter.log_user_filter_results(create_results, to_create) create_errors = create_results.select { |r| r[:status] == :failed } - fail "Creating MUFs resulted in errors: #{create_errors}" if create_errors.any? + if create_errors.any? + sample = create_errors.take(MAX_ERRORS_IN_MESSAGE) + GoodData.logger.error("Creating MUFs failed with #{create_errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") + fail "Creating MUFs resulted in errors, count: #{create_errors.size}, first #{sample.size}: #{sample}" + end end if to_delete.empty? @@ -575,7 +586,11 @@ def self.execute_mufs(user_filters, options = {}) project_log_formatter.log_user_filter_results(delete_results, to_delete) delete_errors = delete_results.select { |r| r[:status] == :failed } if delete_results - fail "Deleting MUFs resulted in errors: #{delete_errors}" if delete_errors&.any? + if delete_errors&.any? + sample = delete_errors.take(MAX_ERRORS_IN_MESSAGE) + GoodData.logger.error("Deleting MUFs failed with #{delete_errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") + fail "Deleting MUFs resulted in errors, count: #{delete_errors.size}, first #{sample.size}: #{sample}" + end end end @@ -643,7 +658,11 @@ def self.execute(user_filters, project_filters, klass, options = {}) maqlify_filters(filters, users, options.merge(users_cache: users_cache, users_must_exist: users_must_exist)) end - fail GoodData::FilterMaqlizationError, errors if !ignore_missing_values && !errors.empty? + if !ignore_missing_values && !errors.empty? + sample = errors.take(MAX_ERRORS_IN_MESSAGE) + GoodData.logger.error("Maqlizing variables failed with #{errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") + fail GoodData::FilterMaqlizationError, "Maqlizing variables resulted in #{errors.size} error(s). First #{sample.size}: #{sample}" + end filters = user_filters.map { |data| client.create(klass, data, project: project) } resolve_user_filters(filters, project_filters) end diff --git a/spec/unit/models/user_filters/user_filter_builder_spec.rb b/spec/unit/models/user_filters/user_filter_builder_spec.rb index 3452b66ac..42d2f9aab 100644 --- a/spec/unit/models/user_filters/user_filter_builder_spec.rb +++ b/spec/unit/models/user_filters/user_filter_builder_spec.rb @@ -199,5 +199,23 @@ expect { subject.execute_mufs(filter_definitions, options) }.to raise_error(/Creating MUFs resulted in errors/) end end + + context 'when creating MUFs results in a large number of errors' do + let(:failed_users) do + Array.new(500) { |i| { 'login' => "user#{i}@example.com", 'detail' => 'x' * 1_000 } } + end + + before do + allow(client).to receive(:post) + .and_return 'userFiltersUpdateResult' => { 'failed' => failed_users } + end + + it 'raises an error with a message bounded to the first few errors' do + expect { subject.execute_mufs(filter_definitions, options) }.to raise_error do |error| + expect(error.message).to match(/Creating MUFs resulted in errors, count: 500, first 10/) + expect(error.message.size).to be < 15_000 + end + end + end end end From 68a17235eb0b6fd37d54fa9cfc06042bb777eeb3 Mon Sep 17 00:00:00 2001 From: Milan Dufek Date: Wed, 29 Jul 2026 14:44:00 +0200 Subject: [PATCH 2/2] fix(lcm): cap per-entry size in bounded error samples Capping only the entry count leaves the message unbounded when a single entry embeds a large value list (CI measured 21KB for 10 entries carrying 1KB details duplicated by the user-hash merge). Render samples through a shared bounded_error_sample helper that truncates each entry to 1000 chars, and assert sampled entries explicitly in the regression test. Addresses CodeRabbit review findings 1 and 3 on PR #2089. JIRA: GRIF-951 Claude-Session: https://claude.ai/code/session_015xTbPyZxxECyx93VgEfW4p --- .../user_filters/user_filter_builder.rb | 22 +++++++++++++------ .../user_filters/user_filter_builder_spec.rb | 3 +++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/gooddata/models/user_filters/user_filter_builder.rb b/lib/gooddata/models/user_filters/user_filter_builder.rb index 758742573..e0f42838d 100644 --- a/lib/gooddata/models/user_filters/user_filter_builder.rb +++ b/lib/gooddata/models/user_filters/user_filter_builder.rb @@ -22,10 +22,18 @@ module UserFilterBuilder @all_domain_users = {} @mutex = Mutex.new - # Cap on error entries embedded in a raised exception's message (and logged). - # Domain-wide syncs can produce one entry per user/value across a whole domain; - # stringifying the raw collection exhausts the JRuby heap (GRIF-951). + # Caps on error entries embedded in a raised exception's message (and logged). + # Domain-wide syncs can produce one entry per user/value across a whole domain, + # and a single entry can embed large value lists; stringifying the raw + # collection exhausts the JRuby heap (GRIF-951). MAX_ERRORS_IN_MESSAGE = 10 + MAX_ERROR_ENTRY_CHARS = 1_000 + + # Renders a bounded sample of an error collection for logs and exception + # messages: caps both the number of entries and each entry's rendered size. + def self.bounded_error_sample(errors) + errors.take(MAX_ERRORS_IN_MESSAGE).map { |e| e.inspect.slice(0, MAX_ERROR_ENTRY_CHARS) } + end # Main Entry function. Gets values and processes them to get filters # that are suitable for other function to process. @@ -493,7 +501,7 @@ def self.execute_mufs(user_filters, options = {}) errors = errors.map do |e| e.merge(pid: project.pid) end - sample = errors.take(MAX_ERRORS_IN_MESSAGE) + sample = bounded_error_sample(errors) GoodData.logger.error("Maqlizing MUFs failed with #{errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") fail GoodData::FilterMaqlizationError, "Maqlizing MUFs resulted in #{errors.size} error(s). First #{sample.size}: #{sample}" end @@ -550,7 +558,7 @@ def self.execute_mufs(user_filters, options = {}) project_log_formatter.log_user_filter_results(create_results, to_create) create_errors = create_results.select { |r| r[:status] == :failed } if create_errors.any? - sample = create_errors.take(MAX_ERRORS_IN_MESSAGE) + sample = bounded_error_sample(create_errors) GoodData.logger.error("Creating MUFs failed with #{create_errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") fail "Creating MUFs resulted in errors, count: #{create_errors.size}, first #{sample.size}: #{sample}" end @@ -587,7 +595,7 @@ def self.execute_mufs(user_filters, options = {}) project_log_formatter.log_user_filter_results(delete_results, to_delete) delete_errors = delete_results.select { |r| r[:status] == :failed } if delete_results if delete_errors&.any? - sample = delete_errors.take(MAX_ERRORS_IN_MESSAGE) + sample = bounded_error_sample(delete_errors) GoodData.logger.error("Deleting MUFs failed with #{delete_errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") fail "Deleting MUFs resulted in errors, count: #{delete_errors.size}, first #{sample.size}: #{sample}" end @@ -659,7 +667,7 @@ def self.execute(user_filters, project_filters, klass, options = {}) end if !ignore_missing_values && !errors.empty? - sample = errors.take(MAX_ERRORS_IN_MESSAGE) + sample = bounded_error_sample(errors) GoodData.logger.error("Maqlizing variables failed with #{errors.size} error(s). First #{sample.size}: #{sample.pretty_inspect}") fail GoodData::FilterMaqlizationError, "Maqlizing variables resulted in #{errors.size} error(s). First #{sample.size}: #{sample}" end diff --git a/spec/unit/models/user_filters/user_filter_builder_spec.rb b/spec/unit/models/user_filters/user_filter_builder_spec.rb index 42d2f9aab..cf3b975dc 100644 --- a/spec/unit/models/user_filters/user_filter_builder_spec.rb +++ b/spec/unit/models/user_filters/user_filter_builder_spec.rb @@ -213,6 +213,9 @@ it 'raises an error with a message bounded to the first few errors' do expect { subject.execute_mufs(filter_definitions, options) }.to raise_error do |error| expect(error.message).to match(/Creating MUFs resulted in errors, count: 500, first 10/) + expect(error.message).to include('user0@example.com') + expect(error.message).to include('user9@example.com') + expect(error.message).not_to include('user10@example.com') expect(error.message.size).to be < 15_000 end end