Skip to content

Commit

Permalink
Add more context to HB when editing via QuickMARC
Browse files Browse the repository at this point in the history
Errors from the QuickMARC API were previously being eaten. This gives us more actionable exceptions and alerts.
  • Loading branch information
mjgiarlo committed Jan 15, 2025
1 parent fba0269 commit 52e393b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 19 deletions.
4 changes: 2 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ Lint/SuppressedException:
Metrics/AbcSize:
Max: 39

# Offense count: 6
# Offense count: 7
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 114
Max: 118

# Offense count: 15
# Configuration parameters: AllowedMethods, AllowedPatterns.
Expand Down
33 changes: 24 additions & 9 deletions app/services/catalog/folio_writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ def initialize(cocina_object:, marc_856_data:)
def save
return if catalog_record_ids.empty? && previous_catalog_record_ids.empty?

Honeybadger.context(
druid: cocina_object.externalIdentifier,
catalog_record_ids:,
previous_catalog_record_ids:
)

# remove 856 for previous catkeys
previous_catalog_record_ids.each { |previous_id| delete_previous_ids(catalog_record_id: previous_id, ignore_not_found: true) }

Expand All @@ -31,10 +37,12 @@ def save
attr_reader :marc_856_data, :cocina_object

def delete_previous_ids(catalog_record_id:, ignore_not_found: false)
FolioClient.edit_marc_json(hrid: catalog_record_id) do |marc_json|
response = FolioClient.edit_marc_json(hrid: catalog_record_id) do |marc_json|
marc_json['fields'].reject! { |field| (field['tag'] == '856') && field['content'].include?(purl_no_protocol) }
end

validate_quickmarc_response!(response)

retry_lookup do
# check that update has completed in FOLIO
raise StandardError, 'PURL still found in instance record after update.' if instance_has_purl?(catalog_record_id:)
Expand All @@ -47,11 +55,13 @@ def delete_previous_ids(catalog_record_id:, ignore_not_found: false)
end

def update_current_ids(catalog_record_id:)
FolioClient.edit_marc_json(hrid: catalog_record_id) do |marc_json|
response = FolioClient.edit_marc_json(hrid: catalog_record_id) do |marc_json|
marc_json['fields'].reject! { |field| (field['tag'] == '856') && field['content'].include?(purl_no_protocol) }
marc_json['fields'] << marc_856_field if ReleaseTagService.released_to_searchworks?(cocina_object:)
end

validate_quickmarc_response!(response)

retry_lookup do
if ReleaseTagService.released_to_searchworks?(cocina_object:)
raise StandardError, 'No matching PURL found in instance record after update.' unless instance_has_purl?(catalog_record_id:)
Expand All @@ -64,24 +74,29 @@ def update_current_ids(catalog_record_id:)
end
end

# @param [Hash,NilClass] response quickmarc response e.g.,
# {"message"=>"Record is too long to be a valid MARC binary record, it's length would be 100106 which is more than 99999 bytes", "type"=>"-4", "code"=>"INTERNAL_SERVER_ERROR"}
def validate_quickmarc_response!(response)
return if response.nil?

Honeybadger.context(quickmarc_response: response)

raise response['message']
end

def retry_lookup
@try_count ||= 0
yield
rescue StandardError => e
@try_count += 1
Rails.logger.warn "Retrying Folio client operation for #{cocina_object.externalIdentifier} (#{@try_count} tries)"

if @try_count <= MAX_TRIES
sleep Settings.catalog.folio.sleep_seconds
retry
end

Honeybadger.notify(
'Error updating Folio record',
error_message: e.message,
context: {
druid: cocina_object.externalIdentifier
}
)
Honeybadger.notify('Error updating Folio record', error_message: e.message)

raise StandardError, 'FOLIO update not completed.'
end
Expand Down
31 changes: 23 additions & 8 deletions spec/services/catalog/folio_writer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@

before do
allow(CocinaObjectStore).to receive(:find).and_return(cocina_object)
allow(FolioClient).to receive(:edit_marc_json).and_yield(folio_response_json)
allow(FolioClient).to receive(:edit_marc_json).and_yield(folio_response_json).and_return(nil)
allow(ReleaseTagService).to receive(:released_to_searchworks?).and_return(release_data)
allow(FolioClient).to receive_messages(fetch_instance_info: instance_record, fetch_marc_hash: source_record)
allow(Honeybadger).to receive(:notify)
Expand Down Expand Up @@ -140,6 +140,24 @@
end
end

context 'when edit operation returns errors' do
let(:error_response) do
JSON.parse({
message: "Record is too long to be a valid MARC binary record, it's length would be 100106 which is more than 99999 bytes",
type: '-4',
code: 'INTERNAL_SERVER_ERROR'
}.to_json)
end

before do
allow(FolioClient).to receive(:edit_marc_json).and_raise(FolioClient::ResourceNotFound).and_return(error_response)
end

it 'raises a RuntimeError with the given response message' do
expect { folio_writer.save }.to raise_error(/Record is too long to be a valid MARC binary record/)
end
end

context 'when existing 856 PURL uses http://' do
let(:folio_response_json) do
{ parsedRecordId: '1ab23862-46db-4da9-af5b-633adbf5f90f',
Expand Down Expand Up @@ -275,8 +293,7 @@
expect(Honeybadger).to have_received(:notify)
.with(
'Error updating Folio record',
error_message: 'No matching PURL found in instance record after update.',
context: { druid: }
error_message: 'No matching PURL found in instance record after update.'
)
.once
end
Expand Down Expand Up @@ -310,8 +327,7 @@
expect(Honeybadger).to have_received(:notify)
.with(
'Error updating Folio record',
error_message: 'More than one matching field with a PURL found on FOLIO record.',
context: { druid: }
error_message: 'More than one matching field with a PURL found on FOLIO record.'
)
.once
end
Expand Down Expand Up @@ -354,8 +370,7 @@
expect(Honeybadger).to have_received(:notify)
.with(
'Error updating Folio record',
error_message: 'PURL still found in instance record after update.',
context: { druid: }
error_message: 'PURL still found in instance record after update.'
)
.once
end
Expand Down Expand Up @@ -459,7 +474,7 @@
before do
allow(FolioClient).to receive(:fetch_instance_info).and_return(instance_record_unreleased)
allow(FolioClient).to receive(:edit_marc_json).with(hrid: 'a8832160').and_raise(FolioClient::ResourceNotFound)
allow(FolioClient).to receive(:edit_marc_json).with(hrid: 'a8832161').and_yield(folio_response_json)
allow(FolioClient).to receive(:edit_marc_json).with(hrid: 'a8832161').and_yield(folio_response_json).and_return(nil)
end

it 'updates MARC record that exists to not include the 856' do
Expand Down

0 comments on commit 52e393b

Please sign in to comment.