Skip to content

Commit

Permalink
BCDA-7811: Update logging to differentiate reasons for job not found (#…
Browse files Browse the repository at this point in the history
…911)

## 🎫 Ticket

https://jira.cms.gov/browse/BCDA-7811

## 🛠 Changes

Added tests + logic to differentiate between a row not being found, and
any other type of issue.

## ℹ️ Context for reviewers

BCDA has received reports of jobs not being found that are clearly in
our database, this will create different levels of logging for those
that are truly not found vs. something else, so we can better address
that.

## ✅ Acceptance Validation

Added unit tests, all tests pass. 

## 🔒 Security Implications

- [ ] This PR adds a new software dependency or dependencies.
- [ ] This PR modifies or invalidates one or more of our security
controls.
- [ ] This PR stores or transmits data that was not stored or
transmitted before.
- [ ] This PR requires additional review of its security implications
for other reasons.

If any security implications apply, add Jason Ashbaugh (GitHub username:
StewGoin) as a reviewer and do not merge this PR without his approval.

---------

Co-authored-by: Alex Dzeda <[email protected]>
  • Loading branch information
alex-dzeda and alex-dzeda authored Feb 16, 2024
1 parent 27a402e commit be7bea0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
12 changes: 7 additions & 5 deletions bcda/api/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,13 @@ func (h *Handler) JobStatus(w http.ResponseWriter, r *http.Request) {

job, jobKeys, err := h.Svc.GetJobAndKeys(r.Context(), uint(jobID))
if err != nil {
logger.Error(err)
// NOTE: This is a catch all and may not necessarily mean that the job was not found.
// So returning a StatusNotFound may be a misnomer
//In contrast to above, if the input COULD be valid, we return a not found header (404)
h.RespWriter.Exception(r.Context(), w, http.StatusNotFound, responseutils.DbErr, "")
if errors.Is(err, sql.ErrNoRows) {
log.API.Info("Requested job not found.", err.Error())
h.RespWriter.Exception(r.Context(), w, http.StatusNotFound, responseutils.DbErr, "Job not found.")
} else {
log.API.Error("Error attempting to request job. Error:", err.Error())
h.RespWriter.Exception(r.Context(), w, http.StatusInternalServerError, responseutils.InternalErr, "Error trying to fetch job. Please try again.")
}

return
}
Expand Down
26 changes: 18 additions & 8 deletions bcda/api/requests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,13 +698,14 @@ func (s *RequestsTestSuite) TestJobStatusErrorHandling() {
requestUrl := v2JobRequestUrl

tests := []struct {
testName string
status models.JobStatus
jobId string
responseHeader int
useMockService bool
timestampOffset int
envVarOverride string
testName string
status models.JobStatus
jobId string
responseHeader int
useMockService bool
timestampOffset int
envVarOverride string
instigateDBFailure bool
}{
{testName: "Invalid jobID (Overflow)",
status: models.JobStatusFailedExpired,
Expand Down Expand Up @@ -735,6 +736,10 @@ func (s *RequestsTestSuite) TestJobStatusErrorHandling() {
status: models.JobStatusCompleted,
jobId: "1",
useMockService: true, responseHeader: http.StatusOK},
{testName: "Simulate unspecified DB Failure",
status: models.JobStatusFailedExpired,
jobId: "1", responseHeader: http.StatusInternalServerError,
useMockService: true, instigateDBFailure: true},
}

resourceMap := s.resourceType
Expand All @@ -745,6 +750,10 @@ func (s *RequestsTestSuite) TestJobStatusErrorHandling() {
if tt.useMockService {
mockSrv := service.MockService{}
timestp := time.Now()
var errResp error = nil
if tt.instigateDBFailure {
errResp = sql.ErrConnDone
}

mockSrv.On("GetJobAndKeys", testUtils.CtxMatcher, uint(1)).Return(
&models.Job{
Expand All @@ -764,8 +773,9 @@ func (s *RequestsTestSuite) TestJobStatusErrorHandling() {
FileName: "testingtesting",
ResourceType: "Patient",
}},
nil,
errResp,
)

h.Svc = &mockSrv

}
Expand Down

0 comments on commit be7bea0

Please sign in to comment.