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

When the connection to database fails, the API should return a valid response #7903

Closed
mattab opened this issue May 13, 2015 · 4 comments
Closed
Assignees
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@mattab
Copy link
Member

mattab commented May 13, 2015

When Piwik cannot connect to the database, the API should return a valid response.

Reproduce:

  • Edit config.ini.php and set a wrong username for the database eg. xyytueyeat
  • Open index.php?module=API&method=API.getPiwikVersion&format=xml
  • Got: HTML error output
  • Expected:
    • HTTP error code 500,
    • and the API call returns a valid eg. XML response that returns the API error reponse eg.
<?xml version="1.0" encoding="utf-8" ?>
<result>
    <error message="Cannot connect to the database: SQLSTATE[28000] [1045] Access denied for user 'xyytueyeat'@'localhost' (using password: YES)" />
</result>

Steps:

  • Add test case
  • Fix issue

Refs #7901 #7902

@saleemkce
Copy link
Contributor

@mattab About "Add test case". I have issues with running test cases from CLI http://forum.piwik.org/read.php?15,126943. There is no problem when web is concerned. Could you show me some sample test case files which closely resembles FrontControllerTest or similar test case file so that I could write test cases properly for this issue?

@mattab
Copy link
Member Author

mattab commented May 24, 2015

Hi @saleemkce

I have issues with running test cases from CLI http://forum.piwik.org/read.php?15,126943

I think the issue is due to the fact you are on Windows. Unfortunately none of the core developers use Windows (we use Ubunty or Mac OS). Feel free to create a separate issue to report that running tests don't work on Windows. Maybe you even have an idea what causes this problem? (eg. maybe it's a path problem as we build paths using forward slash)

@mattab mattab added this to the 2.15.0 milestone Jul 15, 2015
@tsteur
Copy link
Member

tsteur commented Jul 29, 2015

I'm not sure if I understand. Why should we return a valid response in this case? If it's to avoid exposing a message like Access denied for user 'xyytueyeat'@'localhost' (using password: YES) we should maybe rather catch this exception and use a different message?

@mattab
Copy link
Member Author

mattab commented Aug 9, 2015

I'm not sure if I understand. Why should we return a valid response in this case?

The idea is to keep API returning "valid" data at all times, even when Piwik DB is down, so the API is more consistent even in edge cases.

we should maybe rather catch this exception and use a different message?

If you think that's better than showing the message, sounds good. Maybe we could also wrap the actual error message within another generic one eg. Piwik could not connect to your analytics database. Error was %s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

3 participants