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

Handle 429 response code within UI #19233

Closed
JasonMortonNZ opened this issue May 18, 2022 · 6 comments · Fixed by #19433
Closed

Handle 429 response code within UI #19233

JasonMortonNZ opened this issue May 18, 2022 · 6 comments · Fixed by #19433
Assignees
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@JasonMortonNZ
Copy link
Contributor

JasonMortonNZ commented May 18, 2022

At present if a 429 response is returned from an API request the UI would show a generic error message. We would like to provide a more specific custom error message for this response code, indicating the user has sent too many requests in a given amount of time.

This is required to complete the rate limiting feature (dev-2437) on cloud and will improve uptime, cloud performance and customer experience.

@JasonMortonNZ JasonMortonNZ added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label May 18, 2022
@sgiehl sgiehl added this to the For Prioritization milestone Jun 3, 2022
@justinvelluppillai justinvelluppillai added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Jun 8, 2022
@justinvelluppillai
Copy link
Contributor

I will add this to the current milestone as it is a dependency for improving rate limiting on Cloud

@peterhashair peterhashair self-assigned this Jun 20, 2022
@peterhashair
Copy link
Contributor

I am not too sure if I am on the right track here. I think to prepend this before the line. Would that works?

   // load rate limit page if status code 429
        if (http_response_code() === 429) {
            $view = new view('@API/rateLimit');
            return $view->render();
        }

@justinvelluppillai
Copy link
Contributor

I was expecting more likely something in AjaxHelper.ts where it currently handles all errors with 'Something went wrong' (except 504 gets a different message). Maybe worth running this by @bx80 to devise a plan here

@peterhashair
Copy link
Contributor

@JasonMortonNZ just a question on the UI changes, currently the UI shows this error message for the API endpoints
image

  • I could update this message to mention rate-limiting as a possible cause. This will be a simple change.
  • I could detect 429 as a response code and show different messages.

I suggest using option 2. I can use this text there.

image

@sgiehl
Copy link
Member

sgiehl commented Jun 24, 2022

I would suggest to maybe not directly implement any screen or custom message in Matomo itself, as it's quite cloud related only.
It might make more sense to trigger a javascript event whenever a request fails with an unexpected response code.
That way the cloud plugin could hook into that event and display a custom message.
This would later also allow to handle any other response code, we might need to introduce on cloud

@tsteur
Copy link
Member

tsteur commented Jun 25, 2022

I could detect 429 as a response code and show different messages.

This would be ideal @peterhashair . We would want to handle the 429 error directly in core and then show a message. However, the message will need to be different so it works for regular On-Premise users too. The Cloud team will then maybe want to overwrite the message/translation.

I would suggest to maybe not directly implement any screen or custom message in Matomo itself, as it's quite cloud related only.

The idea was to have this in core so that other On-Premise users can benefit from this as well as some would already have some rate limiting configured and as always we would then ideally also document this behaviour in an FAQ so making it more clear when others set it up in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants