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

Matomo should throw an error if the request to api.matomo.org fails #12724

Open
Findus23 opened this issue Apr 13, 2018 · 7 comments
Open

Matomo should throw an error if the request to api.matomo.org fails #12724

Findus23 opened this issue Apr 13, 2018 · 7 comments
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently.

Comments

@Findus23
Copy link
Member

After long debugging of an issue where no update was found, it turned out that a proxy was blocking the request.
https://forum.matomo.org/t/update-from-3-3-0-to-3-4-0-not-offered-in-matomo/27786

In my opinion the updater should not act as if no update is available if the request fails.

On the contrary always showing warnings may be annoying in case api.matomo.org is down once. Maybe a warning could only be shown after repeated retries.

@Findus23 Findus23 added the c: Usability For issues that let users achieve a defined goal more effectively or efficiently. label Apr 13, 2018
@fdellwing
Copy link
Contributor

fdellwing commented Apr 16, 2018

The Http::sendHttpRequest() function already handles http errors and returns false. So there needs to be some other problem. Maybe squid does not correctly answer with http status 403?

To prevent this issue on Matomo site, a new function should be added to core/Http.php like this:

public static function sendHttpRequestWithExpectedResult($aUrl,
                                           $expectedPattern,
                                           $timeout,
                                           $userAgent = null,
                                           $destinationPath = null,
                                           $followDepth = 0,
                                           $acceptLanguage = false,
                                           $byteRange = false,
                                           $getExtendedInfo = false,
                                           $httpMethod = 'GET', 
                                           $httpUsername = null,
                                           $httpPassword = null)
    {
        // Do the same as in sendHttpRequest but do a preg_match() against the body afterwards.
    }

If you like this solution I'll provide a PR for that.

@fdellwing
Copy link
Contributor

Any opinions?

@tsteur
Copy link
Member

tsteur commented May 29, 2018

Maybe we could add a system check for it? To test we receive a version number?

@Findus23
Copy link
Member Author

@tsteur That won't help people who have a proxy blocking the request without their knowledge.

@tsteur
Copy link
Member

tsteur commented May 29, 2018

Sorry not sure I get it. I read the forum post... as we probably don't really want to fail during run time when that request fails, it may be better to show it in a system check (which is also shown on Admin Home) that something is wrong / not working?

@Findus23
Copy link
Member Author

Ah, sorry my mistake. A system check is a great idea. (I read system test, which wouldn't make any sense)

@fdellwing
Copy link
Contributor

fdellwing commented May 30, 2018

👍

That check should also allow to see the returned error message if there is one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently.
Projects
None yet
Development

No branches or pull requests

4 participants