Skip to content

chore: update db code to return db errors instead of api errors#558

Open
omarelkashef wants to merge 1 commit into
canonical:mainfrom
omarelkashef:WD-37141-db-errors
Open

chore: update db code to return db errors instead of api errors#558
omarelkashef wants to merge 1 commit into
canonical:mainfrom
omarelkashef:WD-37141-db-errors

Conversation

@omarelkashef

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the database store layer to return store-specific sentinel errors (e.g., not-found / already-exists) rather than HTTP/API-specific status errors, and wires those store errors into LXD’s response.SmartError mapping during service startup.

Changes:

  • Replace api.StatusErrorf/StatusErrorCheck usage in internal/pkg/database/store with store.ErrNotFound/store.ErrAlreadyExists wrappers and store.IsNotFound checks.
  • Add internal/pkg/database/store/errors.go to centralize store error definitions and helper constructors.
  • Initialize response.SmartError mappings at startup in both management-api and cluster-connector.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/pkg/database/store/sessions.go Switch session store functions from HTTP status errors to store sentinel errors.
internal/pkg/database/store/remote_cluster.go Switch remote cluster store functions to store sentinel errors and checks.
internal/pkg/database/store/remote_cluster_token.go Switch remote cluster token store functions to store sentinel errors and checks.
internal/pkg/database/store/remote_cluster_detail.go Switch remote cluster detail store functions to store sentinel errors and checks.
internal/pkg/database/store/identities.go Switch identity store functions to store sentinel errors and checks.
internal/pkg/database/store/errors.go Introduce ErrNotFound/ErrAlreadyExists plus helpers (NotFoundErrorf, etc.).
internal/pkg/api/api.go Add InitializeSmartErrorMappings() to map store sentinel errors to HTTP statuses for SmartError.
internal/app/management-api/core/auth/session.go Update auth session logic to use store.IsNotFound instead of LXD shared API status checks.
internal/app/management-api/core/auth/oidc.go Update OIDC verifier to use store.IsNotFound when handling missing sessions.
cmd/management-api/management-api.go Call api.InitializeSmartErrorMappings() during service startup.
cmd/cluster-connector/cluster-connector.go Call api.InitializeSmartErrorMappings() during service startup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/pkg/database/store/errors.go
Comment thread internal/pkg/database/store/errors.go
Signed-off-by: Omar Elkashef <omarelkashef01@gmail.com>

@edlerd edlerd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this is the right thing to do. It seems an established pattern also in LXD to return http statuses from the database layer. An example is in profiles.mapper.go. To me it seems okay, and not fundamentally wrong, also it seems simpler.

Shall we keep things as they are right now?

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.

3 participants