Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/decode gitlab error #246

Merged
merged 3 commits into from
Jan 20, 2025
Merged

Feat/decode gitlab error #246

merged 3 commits into from
Jan 20, 2025

Conversation

puffitos
Copy link
Collaborator

Motivation

Relates to #231

Changes

Added a general error struct, in which we can decode API error messages and the non-OK status, to return as an error. Additionally, I've added contexts to the target manager's logging calls.

Tests done

Recreated a case which can create this POST error, by creating a file which has the same DNS name as my dev sparrow b
but a different URL scheme (http vs https). Getting the following logs now (yes, I know we have duplicate logs, but this shouldn't be addressed in this PR):

time=19:26:03 level=ERROR source=/Users/puffito/dev/repos/sparrow/pkg/sparrow/targets/remote/gitlab/gitlab.go:321 msg="Failed to post file" status="400 Bad Request"
time=19:26:03 level=ERROR source=/Users/puffito/dev/repos/sparrow/pkg/sparrow/targets/manager.go:207 msg="Failed to register global gitlabTargetManager" error="gitlab API sent an unexpected status code (400) with the following error message: {\"message\":\"A file with this name already exists\"}"
time=19:26:03 level=WARN source=/Users/puffito/dev/repos/sparrow/pkg/sparrow/targets/manager.go:127 msg="Failed to register self as global target" error="gitlab API sent an unexpected status code (400) with the following error message: {\"message\":\"A file with this name already exists\"}"
  • Unit tests succeeded
  • E2E tests succeeded

TODO

  • I've assigned this PR to myself
  • I've labeled this PR correctly

@puffitos puffitos added the area/target-manager Issues/PRs related to the TargetManager label Dec 25, 2024
@puffitos puffitos self-assigned this Dec 25, 2024
Copy link
Collaborator

@niklastreml niklastreml left a comment

Choose a reason for hiding this comment

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

LGTM, just wondering why you're not exposing the apiError type and chose to keep it private

@puffitos
Copy link
Collaborator Author

LGTM, just wondering why you're not exposing the apiError type and chose to keep it private

Not needed anywhere else and it's extremely specific to the gitlab specification. Looks like a pretty solid candidate for an unexposed type. 😊

@puffitos puffitos merged commit 86ed6cd into main Jan 20, 2025
8 checks passed
@puffitos puffitos deleted the feat/decode-gitlab-error branch January 20, 2025 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/target-manager Issues/PRs related to the TargetManager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants