VR&E - review the current job process for bug fixes/improvements

[va-albers]

The current VR&E create process starts in the app/controllers/v0/veteran_readiness_employment_claims_controller.rb controller, then calls the app/sidekiq/vre/submit1900_job.rb sidekiq job to complete the activity. Please take a look at the following items:

• [ ] The sidekiq job pulls user state from User, which is based on redis. This can cause failures when User state goes out of cache. Switch to using a persistent state for the asynchronous job. [high]
• [ ] traditionally a Sidekiq job will try to call perform(), and it will re-call perform() again for retryable issues by throwing an exception, until it fails enough times to call the sidekiq_retries_exhausted where it will run the backup path. In veteran_readiness_employment_claim.rb, around line 118, the code immediately calls the backup path if the upload_to_vbms job fails one time. This bypasses the Sidekiq retry mechanism. Generally the backup path for retryable issues should happen in the sidekiq_retries_exhausted handler so the job can try the main path multiple times? In this current configuration I'm not sure if the exhausted handler would be called even if the remote system was unavailable. [high]
• [ ] wherever the "user = User.find user_uuid" line ends up (currently in submit1900_job, but it should move out of sidekiq), verify that it returns a real user object before using it. Currently I think it is passing null to a few different methods with unexpected results because of expired cache. [high]
• [ ] In the current state I believe that claims cloud silently fail (since it appears that the system doesn't use the traditional retry mechanism).
• [ ] veteran_readiness_employments_claims_controller.rb. It doesn't appear that the claims.save false case writes any context to Datadog logs. Consider using claim.errors/claim.errors.full_messages or something equivalent (no PII though) to write failure information out to Dd. [medium]
• [ ] - consider the utility of the Rails.logger.info call on line 14 of veteran_readiness_employment_claims_controller.rb. Is this useful? [minor]
• [ ] consider the utility of logging a uuid VRE claim office location on line 175 of app/models/saved_claim/veteran_readiness_employment_claim.rb. Is this useful? [minor]
• [ ] Look at places that are writing Sentry-focused logs and consider making them more Datadog-friendly (don't use Sentry-specific extensions, consider using JSON log format for better searchability)

[va-albers]

One example of where redis User could at least be limiting our visibility/veteran awareness:

• submit1900_job.rb->perform line 24 can claim.send_to_vre(user) with an null user
• if user is null the system posts a Sentry message "Participant id is blank when submitting VRE claim" (which is technically true). Sentry currently shows 474 of these errors in the last 14 days link
• Note: the send_to_central_mail!(user) call on line 122 (in veteran_readiness_employment_claim.rb) has a rescue that logs an error, but the comparable send_to_central_mail!(user) on line 131 does not? I think the result of this is that the method will fail with an uncaught exception if this occurs?
• the null user object is sent to send_to_central_mail (line 131), which passes it to send_central_mail_confirmation_email(user) on line 169 which does a "return if user.va_profile_email.blank?" without logging - suggesting that a null user exception would die here with no notification sent to the veteran?

[va-albers]

In this case above the null user means that it skips the upload_to_vbms(), send_vbms_confirmation_email() and/or send_to_central_mail() calls entirely, going straight to the send_vre_email_form() call. Is this acceptable?

[va-albers]

This relates to monitor DD-186965