Skip to content

Lookout: wrap ErrNotFound in GetJobSpec not-found error#4773

Open
mauriceyap wants to merge 1 commit intomasterfrom
get-job-spec-errnotfound
Open

Lookout: wrap ErrNotFound in GetJobSpec not-found error#4773
mauriceyap wants to merge 1 commit intomasterfrom
get-job-spec-errnotfound

Conversation

@mauriceyap
Copy link
Copy Markdown
Collaborator

Replace github.com/pkg/errors with stdlib fmt.Errorf and wrap ErrNotFound so callers can use errors.Is to distinguish not-found from other errors.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR modernises the error handling in GetJobSpec: it drops the github.com/pkg/errors dependency in favour of the stdlib, adopts errors.Is for the pgx.ErrNoRows sentinel check, and wraps the package-level ErrNotFound sentinel into the returned error so callers can reliably use errors.Is(err, repository.ErrNotFound) to distinguish not-found from other database errors.

  • getjobspec.go: imports swapped (github.com/pkg/errors"errors" + "fmt"), equality check upgraded to errors.Is(err, pgx.ErrNoRows), error now wraps ErrNotFound via %w — matches the pattern in getjoberror.go and getjobrunerror.go.
  • getjobspec_test.go: test strengthened from assert.Error to assert.ErrorIs(t, err, ErrNotFound), correctly validating the new sentinel wrapping.
  • No logic regressions; the pgx.ErrNoRows guard and the pass-through return nil, err for all other DB errors are both preserved.

Confidence Score: 5/5

  • This PR is safe to merge — it is a small, focused, and correct error-handling improvement with no logic regressions.
  • The change is minimal: two files, a dependency swap from github.com/pkg/errors to stdlib, an idiomatic errors.Is upgrade, and sentinel wrapping. The pgx.ErrNoRows guard and pass-through for other errors are both preserved. The implementation is consistent with the already-reviewed sibling files (getjoberror.go, getjobrunerror.go), and the test assertion is correctly strengthened to validate the new behaviour. No other behaviour is affected.
  • No files require special attention.

Important Files Changed

Filename Overview
internal/lookout/repository/getjobspec.go Replaces github.com/pkg/errors with stdlib errors/fmt, switches to idiomatic errors.Is(err, pgx.ErrNoRows), and wraps ErrNotFound so callers can use errors.Is — consistent with sibling files getjoberror.go and getjobrunerror.go.
internal/lookout/repository/getjobspec_test.go Updates the not-found test assertion from the weaker assert.Error to assert.ErrorIs(t, err, ErrNotFound), correctly verifying the sentinel wrapping introduced in the main file.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GetJobSpec called] --> B[QueryRow: SELECT job_spec\nWHERE job_id = $1]
    B --> C{err != nil?}
    C -- No --> D[Decompress rawBytes]
    D --> E{decompress err?}
    E -- No --> F[proto.Unmarshal]
    F --> G{unmarshal err?}
    G -- No --> H[Return *api.Job]
    G -- Yes --> I[Return unmarshal error]
    E -- Yes --> J[Return decompress error]
    C -- Yes --> K{errors.Is\nerr, pgx.ErrNoRows?}
    K -- Yes --> L["Return nil,\nfmt.Errorf('...not found: %w', ErrNotFound)"]
    K -- No --> M[Return nil, err\n other DB error]
Loading

Reviews (7): Last reviewed commit: "Lookout: wrap ErrNotFound in GetJobSpec ..." | Re-trigger Greptile

@mauriceyap mauriceyap enabled auto-merge (squash) March 17, 2026 17:09
@mauriceyap mauriceyap force-pushed the get-job-spec-errnotfound branch from ac5ab28 to e0b4388 Compare March 17, 2026 17:58
@mauriceyap
Copy link
Copy Markdown
Collaborator Author

@Mergifyio refresh

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 20, 2026

refresh

✅ Pull request refreshed

@mauriceyap mauriceyap disabled auto-merge March 20, 2026 16:09
@mauriceyap
Copy link
Copy Markdown
Collaborator Author

@Mergifyio rebase

@mauriceyap
Copy link
Copy Markdown
Collaborator Author

@Mergifyio refresh

@mauriceyap
Copy link
Copy Markdown
Collaborator Author

https://github.com/Mergifyio refresh

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 23, 2026

refresh

✅ Pull request refreshed

@pavlovic-ivan
Copy link
Copy Markdown
Collaborator

https://github.com/Mergifyio refresh

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 23, 2026

refresh

☑️ Command disallowed due to command restrictions in the Mergify configuration.

Details
  • any of:
    • sender = {{author}}
    • sender-permission >= write

Replace github.com/pkg/errors with stdlib fmt.Errorf and wrap ErrNotFound
so callers can use errors.Is to distinguish not-found from other errors.

Signed-off-by: Maurice Yap <mauriceyap@hotmail.co.uk>
@mauriceyap mauriceyap force-pushed the get-job-spec-errnotfound branch from 720db59 to ec4ed20 Compare March 23, 2026 10:31
@mauriceyap
Copy link
Copy Markdown
Collaborator Author

@Mergifyio refresh

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 23, 2026

refresh

✅ Pull request refreshed

@mauriceyap
Copy link
Copy Markdown
Collaborator Author

@Mergifyio refresh

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 23, 2026

refresh

✅ Pull request refreshed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants