add transmittable error for xml conversion#302
Conversation
📝 WalkthroughWalkthroughReplace the direct Changes
Sequence Diagram(s)sequenceDiagram
participant AR as ApplicantRequest
participant IRTX as InitialRequestToXml
participant EH as ErrorHandler
AR->>IRTX: call(initial_verification_request)
alt XML Generation Success
IRTX-->>AR: Success(result)
AR->>AR: return success result
else XML Generation Failure
IRTX-->>AR: Failure(error_message)
AR->>EH: handle_failure(context, error_message)
EH-->>AR: Failure response (propagate)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb:
- Around line 58-68: The generate_xml_and_schema_validate method currently only
handles Failure monads from
AcaEntities::Serializers::Xml::Fdsh::Vlp::H92::Operations::InitialRequestToXml.new.call;
wrap that call in a begin/rescue (similar to
generate_initial_verification_request) to catch exceptions, call
handle_failure(:generate_xml_and_schema_validate, { transaction:
@request_transaction, transmission: @request_transmission, job: @job },
"Exception generating VLP request XML: #{e.message}") in the rescue, and ensure
you return an appropriate Failure-like result after logging so exceptional
errors are tracked the same as returned failures.
In @spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb:
- Around line 200-215: Add assertions in the "due to schema validation failure"
example to verify statuses are updated to :failed (like the XML serialization
failure context). After calling subject.call(params) (the existing result =
subject.call(params)), assert that the relevant status fields on the VLP request
and related entities (the same status accessors checked in the XML serialization
failure test) are set to :failed; reuse the same expectations/attribute
accessors used in the XML serialization failure context to keep coverage
consistent with the existing spec.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rbspec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-15T16:28:40.423Z
Learnt from: vkghub
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:119-123
Timestamp: 2025-07-15T16:28:40.423Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the application hash structure guarantees that top level keys like `:person_name` and `:demographics` will always be present, so direct hash access in `name_and_dob_match?` method is safe and doesn't require defensive navigation using `dig`.
Applied to files:
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
📚 Learning: 2025-07-16T13:34:53.451Z
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:129-138
Timestamp: 2025-07-16T13:34:53.451Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.
Applied to files:
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
📚 Learning: 2025-07-14T19:06:48.270Z
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb:99-119
Timestamp: 2025-07-14T19:06:48.270Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.
Applied to files:
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb
🧬 Code graph analysis (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
app/operations/fdsh/ssa_vlp/rj3/request_processor_utils.rb (1)
handle_failure(73-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-upload-image
- GitHub Check: rspec
TristanB17
left a comment
There was a problem hiding this comment.
Nice specs, looks good, always a fan of better logging
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1)
173-243: Consider extracting a shared example to reduce duplication.Both
'due to XML serialization failure'and'due to schema validation failure'contexts have nearly identical test structures, differing only in the failure message. Following the pattern established byhandles transformation failure(lines 101-122), you could reduce duplication with a shared example.🔎 Proposed shared example extraction
shared_examples 'handles xml generation failure' do |expected_failure_message| before do allow(AcaEntities::Serializers::Xml::Fdsh::Vlp::H92::Operations::InitialRequestToXml).to receive(:new).and_return(mock_xml_operation) allow(mock_xml_operation).to receive(:call).and_return(Dry::Monads::Result::Failure.new(failure_message)) end it 'returns failure with error message' do result = subject.call(params) expect(result).to be_failure expect(result.failure).to eq(expected_failure_message) end it 'updates the request transaction and transmission status to failed' do subject.call(params) request_transmission = Transmittable::Transmission.find_by(key: :vlp_verification_request) request_transaction = Transmittable::Transaction.find_by(key: :vlp_verification_request) expect(request_transmission.process_status.latest_state).to eq(:failed) expect(request_transaction.process_status.latest_state).to eq(:failed) end it 'logs the failure' do subject.call(params) request_transmission = Transmittable::Transmission.find_by(key: :vlp_verification_request) request_transaction = Transmittable::Transaction.find_by(key: :vlp_verification_request) expect(request_transmission.transmittable_errors.last.message).to include(failure_message) expect(request_transaction.transmittable_errors.last.message).to include(failure_message) end end # Usage: context 'due to XML serialization failure' do let(:failure_message) { 'XML serialization failed' } let(:mock_xml_operation) { instance_double(AcaEntities::Serializers::Xml::Fdsh::Vlp::H92::Operations::InitialRequestToXml) } include_examples 'handles xml generation failure', "Failed to generate VLP request XML due to XML serialization failed" end context 'due to schema validation failure' do let(:failure_message) { 'Schema validation failed - invalid XML structure' } let(:mock_xml_operation) { instance_double(AcaEntities::Serializers::Xml::Fdsh::Vlp::H92::Operations::InitialRequestToXml) } include_examples 'handles xml generation failure', "Failed to generate VLP request XML due to Schema validation failed - invalid XML structure" end
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-16T13:34:53.451Z
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb:129-138
Timestamp: 2025-07-16T13:34:53.451Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/vlp/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.
Applied to files:
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
📚 Learning: 2024-10-08T21:02:47.798Z
Learnt from: saikumar9
Repo: ideacrew/fdsh_gateway PR: 219
File: app/models/alive_status/family.rb:45-78
Timestamp: 2024-10-08T21:02:47.798Z
Learning: The class `::AcaEntities::Operations::CreateFamily` is well-tested within its own gem, and its usage in the methods `request_family_entity` and `response_family_entity` of the `AliveStatus::Family` model is sufficiently covered in `spec/models/alive_status/family_spec.rb`.
Applied to files:
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
📚 Learning: 2025-07-14T19:06:48.270Z
Learnt from: ymhari
Repo: ideacrew/fdsh_gateway PR: 268
File: app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb:99-119
Timestamp: 2025-07-14T19:06:48.270Z
Learning: In `app/operations/fdsh/ssa_vlp/rj3/ssa/process_verification_response.rb`, the evidence appending approach in `update_evidences_with_failure` and `update_evidences_with_results` methods is intentionally designed to accumulate evidence entries rather than replace them, as confirmed by ymhari.
Applied to files:
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb
🧬 Code graph analysis (1)
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1)
app/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request.rb (1)
call(15-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-upload-image
- GitHub Check: rspec
🔇 Additional comments (1)
spec/operations/fdsh/ssa_vlp/rj3/vlp/applicant_request_spec.rb (1)
168-244: Well-structured failure path coverage.The new test contexts comprehensively cover XML generation and schema validation failure scenarios. Each context properly verifies:
- The result is a failure with the expected message format
- Both transmission and transaction statuses are updated to
:failed- Error messages are logged to
transmittable_errorsThis aligns well with the production code's explicit failure handling in
generate_xml_and_schema_validate.
* optimize 1095a csv report to run faster (#299) * optimize 1095a csv report to run faster * update output file names * fix rubocop issues * optimize h41 run time (#300) * add transmittable error (#302) * add script to clean up 2026 h36 irs groups (#304) * add script to clean up 2026 h36 irs groups * fix rubocop issues * add dependent destroy for other associations * handle exceptions * fix comments in the script * improve performance by using delete_all instead of destroy (#305) --------- Co-authored-by: Sai Praveen Gudimetla <saipraveen.gudimetla@gmail.com> Co-authored-by: Jacob Kagon <69021620+jacobkagon@users.noreply.github.com>
https://app.clickup.com/t/868gve9fh
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.