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

SQL importer web UI should return the DatabaseServiceException message instead of generic error #4007

Open
braiam opened this issue Jun 23, 2021 · 11 comments
Labels
error handling Improving the ways errors are reported to users SQL/database Integration with SQL databases in general Type: Feature Request Identifies requests for new features or enhancements. These involve proposing new improvements.

Comments

@braiam
Copy link

braiam commented Jun 23, 2021

I use openrefine to import projects directly from a database to process it and exporting as a file to share with others. I created a custom user for this to reduce the possibility of a bug or attack vector that would modify/damage the database. That means that I have to explicitly grant permissions on tables/schemas. Sometimes I forget this step, and the logs accurately reflect this issue:

refine[730]: com.google.refine.extension.database.DatabaseServiceException: ERROR: permission denied for table name_table

But the message in the web UI is just a generic "error:Bad Request" without further details.

Proposed solution

If there's an error, the top error message (without the at details) should be returned along the "error:Bad Request" via the UI, so that users are informed of the issue and work out the solution. It would provide immediate actionable feedback of what is wrong with the query, so they can fix it.

Alternatives considered

While the error logs do point out the problem, in case of a shared instance where there isn't an easy way to access the box where openrefine resides, it would take time to figure out if the SQL query is correct, login the remote box, pull the logs, find the specific message, and fix the issue with the database.

@braiam braiam added Type: Feature Request Identifies requests for new features or enhancements. These involve proposing new improvements. Status: Pending Review Indicates that the issue or pull request is awaiting review by project maintainers or collaborators labels Jun 23, 2021
@thadguidry
Copy link
Member

Agreed, we should probably propagate the real error message so that it's more useful.
@braiam Do we also need to make any changes to the message box UI itself? centering, text size, anything else?

@braiam
Copy link
Author

braiam commented Jun 23, 2021

Not really, just more information about the error.

@wetneb wetneb added error handling Improving the ways errors are reported to users Good First Issue Indicates issues suitable for newcomers to design or coding, providing a gentle introduction. SQL/database Integration with SQL databases in general and removed Status: Pending Review Indicates that the issue or pull request is awaiting review by project maintainers or collaborators labels Jun 23, 2021
@aileenpalafox
Copy link

Hi, first time working on this project. Can I give it a try? @braiam

@braiam
Copy link
Author

braiam commented Jun 29, 2021

@aileenpalafox this is a open source project, so break a leg!

@thadguidry
Copy link
Member

@aileenpalafox Thanks! Let us know in this issue if you have questions or you can ask on our dev mailing list

@aileenpalafox aileenpalafox removed their assignment Jul 12, 2021
@aileenpalafox
Copy link

aileenpalafox commented Jul 12, 2021

Hello, again @thadguidry I faced some problems when trying to solve this one and ran out of time so I'm gonna drop it for now. In case anyone else want to work on it they can give it a try.

@thadguidry
Copy link
Member

@aileenpalafox No problem, perhaps you can work on another issue we have marked with a label called "good first issue"? If not and you are too busy, we certainly understand and thanks anyways for offering to help!

@aileenpalafox
Copy link

Thank you for the offer @thadguidry, right now I've other stuff going on so I'll have to pass. Good luck with the project!

@wetneb
Copy link
Sponsor Member

wetneb commented Aug 31, 2021

Posted by @bhargavii on our Gitter channel

I'm new to this project, so was looking into issues with good first issue tag,I came across the "#4007 SQL importer web UI should return the DatabaseServiceException message instead of generic error" issue ,
when I tried replicating the error, the message in the web UI isn't generic "error:Bad Request", instead it is specifying the corresponding SQL query followed by "command denied to user".
So I believe this issue doesn't persist anymore.
If it does persist, can I know which folders do I have to look into in order to resolve this.

@wetneb wetneb added Status: Needs More Information Indicates issues that lack sufficient information for the project team to act upon. and removed Good First Issue Indicates issues suitable for newcomers to design or coding, providing a gentle introduction. labels Aug 31, 2021
@braiam
Copy link
Author

braiam commented Aug 31, 2021

@wetneb Are we sure? I'm using version 3.5-beta1 [1b9907e] and still can reproduce the issue. Is using master recommended? With a postgresql database:

Aug 31 06:23:48 dtbhome refine[766]: 06:23:48.894 [         TestQueryCommand] TestQueryCommand::Post::DatabaseServiceException::{} (4ms)
Aug 31 06:23:48 dtbhome refine[766]: com.google.refine.extension.database.DatabaseServiceException: ERROR: permission denied for table cerdef2021
Aug 31 06:23:48 dtbhome refine[766]:         at com.google.refine.extension.database.pgsql.PgSQLDatabaseService.testQuery(PgSQLDatabaseService.java:229)
Aug 31 06:23:48 dtbhome refine[766]:         at com.google.refine.extension.database.cmd.TestQueryCommand.doPost(TestQueryCommand.java:82)
Aug 31 06:23:48 dtbhome refine[766]:         at com.google.refine.RefineServlet.service(RefineServlet.java:187)
Aug 31 06:23:48 dtbhome refine[766]:         at javax.servlet.http.HttpServlet.service(HttpServlet.java:750)

@wetneb wetneb removed the Status: Needs More Information Indicates issues that lack sufficient information for the project team to act upon. label Aug 31, 2021
@wetneb
Copy link
Sponsor Member

wetneb commented Aug 31, 2021

@braiam thanks for the confirmation! I don't know if @bhargavii is still interested in working on this, perhaps this additional information will help them understand how to reproduce the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Improving the ways errors are reported to users SQL/database Integration with SQL databases in general Type: Feature Request Identifies requests for new features or enhancements. These involve proposing new improvements.
Projects
None yet
Development

No branches or pull requests

4 participants