Commit 1b015620 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Recover from errors when a parent is not preloaded

parent d8dd75ca
...@@ -37,7 +37,20 @@ module GroupDescendant ...@@ -37,7 +37,20 @@ module GroupDescendant
parent ||= preloaded.detect { |possible_parent| possible_parent.is_a?(Group) && possible_parent.id == child.parent_id } parent ||= preloaded.detect { |possible_parent| possible_parent.is_a?(Group) && possible_parent.id == child.parent_id }
if parent.nil? && !child.parent_id.nil? if parent.nil? && !child.parent_id.nil?
raise ArgumentError.new('parent was not preloaded') parent = child.parent
exception = ArgumentError.new <<~MSG
parent: [GroupDescendant: #{parent.inspect}] was not preloaded for [#{child.inspect}]")
This error is not user facing, but causes a +1 query.
MSG
extras = {
parent: parent,
child: child,
preloaded: preloaded.map(&:full_path)
}
issue_url = 'https://gitlab.com/gitlab-org/gitlab-ce/issues/40785'
Gitlab::Sentry.track_exception(exception, issue_url: issue_url, extra: extras)
end end
if parent.nil? && hierarchy_top.present? if parent.nil? && hierarchy_top.present?
......
---
title: Show shared projects on group list
list
merge_request: 18390
author:
type: fixed
...@@ -79,9 +79,24 @@ describe GroupDescendant, :nested_groups do ...@@ -79,9 +79,24 @@ describe GroupDescendant, :nested_groups do
expect(described_class.build_hierarchy(groups)).to eq(expected_hierarchy) expect(described_class.build_hierarchy(groups)).to eq(expected_hierarchy)
end end
it 'tracks the exception when a parent was not preloaded' do
expect(Gitlab::Sentry).to receive(:track_exception).and_call_original
expect { GroupDescendant.build_hierarchy([subsub_group]) }.to raise_error(ArgumentError)
end
it 'recovers if a parent was not reloaded by querying for the parent' do
expected_hierarchy = { parent => { subgroup => subsub_group } }
# this does not raise in production, so stubbing it here.
allow(Gitlab::Sentry).to receive(:track_exception)
expect(GroupDescendant.build_hierarchy([subsub_group])).to eq(expected_hierarchy)
end
it 'raises an error if not all elements were preloaded' do it 'raises an error if not all elements were preloaded' do
expect { described_class.build_hierarchy([subsub_group]) } expect { described_class.build_hierarchy([subsub_group]) }
.to raise_error('parent was not preloaded') .to raise_error(/was not preloaded/)
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment