Skip to content

Conversation

@gacevicljubisa
Copy link
Member

@gacevicljubisa gacevicljubisa commented Jul 9, 2025

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Adds handler that checks if redistributionAgent is initialized.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Redistribution handler panics when called with storage incentives disabled #5143

Screenshots (if appropriate):

@gacevicljubisa gacevicljubisa marked this pull request as ready for review July 9, 2025 10:18
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

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

seems good just my 2 cents

func (s *Service) checkStorageIncentivesAvailability(handler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if s.redistributionAgent == nil {
jsonhttp.NotImplemented(w, "Storage incentives are disabled. This endpoint is unavailable.")
Copy link
Member

Choose a reason for hiding this comment

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

well, it is implemented, it is more like a 404 Not Found or a 403 Forbidden. I would simply go with 404

Copy link
Member Author

Choose a reason for hiding this comment

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

We have used this status code when Swap is disabled and Chequebook is disabled, which was done in this PR. But we can change it to 403 Forbidden, because route does exit, but it is disabled on server side. In that case we should change this for all 3 endpoints to have the same behaviour.
@janos, @nugaon any opion on this?

Copy link
Member

Choose a reason for hiding this comment

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

403 is also fine from my side on all similar places

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

@gacevicljubisa gacevicljubisa merged commit 930110d into master Jul 23, 2025
15 checks passed
@gacevicljubisa gacevicljubisa deleted the fix_redis-handler branch July 23, 2025 12:59
@bcsorvasi bcsorvasi added this to the v2.7.0 milestone Oct 8, 2025
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.

Redistribution handler panics when called with storage incentives disabled

7 participants