Skip to content

chore(api-error): change unauthorized msg and add tests #2190

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

Merged
merged 2 commits into from
May 28, 2025

Conversation

JGAntunes
Copy link
Member

@JGAntunes JGAntunes commented May 27, 2025

What this PR does / why we need it:

Story: https://app.shortcut.com/replicated/story/124146/add-tests-to-api-error-types

Adds tests to api/types/errors_test.go and changes the Unauthorized API error to not expose the underlying err.Error() through Message.

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?


Does this PR require documentation?

Copy link

github-actions bot commented May 27, 2025

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-f4060de" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-f4060de?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

emosbaugh
emosbaugh previously approved these changes May 27, 2025
sgalsaleh
sgalsaleh previously approved these changes May 27, 2025
@sgalsaleh
Copy link
Member

Merging into a branch.

@sgalsaleh sgalsaleh force-pushed the salah/sc-123874/steelthread-configure-data-dir-in-the-new branch 2 times, most recently from ec30def to 50ef28e Compare May 27, 2025 14:49
Base automatically changed from salah/sc-123874/steelthread-configure-data-dir-in-the-new to main May 27, 2025 18:35
@sgalsaleh sgalsaleh dismissed stale reviews from emosbaugh and themself May 27, 2025 18:35

The base branch was changed.

@JGAntunes
Copy link
Member Author

I don't think this has been merged @sgalsaleh. Rebased with main, so should be good to 👀 and ✅

@JGAntunes JGAntunes self-assigned this May 28, 2025
@JGAntunes JGAntunes requested review from sgalsaleh and emosbaugh May 28, 2025 09:26
@@ -57,7 +57,7 @@ func NewBadRequestError(err error) *APIError {
func NewUnauthorizedError(err error) *APIError {
return &APIError{
StatusCode: http.StatusUnauthorized,
Message: err.Error(),
Message: "Unauthorized",
Copy link
Member Author

Choose a reason for hiding this comment

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

When dealing with 401's we shouldn't be exposing any context around what happened.

@emosbaugh
Copy link
Member

Test failures seem unrelated. Merging

@emosbaugh emosbaugh merged commit 217ec38 into main May 28, 2025
52 of 64 checks passed
@emosbaugh emosbaugh deleted the chore/api-errors branch May 28, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants