Better error message when directory containing database file is not writeable

[sampsyo]

Following up from #1664: We should exit gracefully with a useful error message if we get an OperationalError from SQLite that indicates a permissions problem on write.

[Mary011196]

Hello,
I am interested in contributing to this issue. I've already add some lines of code in file library.py but i don't know if this is what you want. Please explain me a little more this issue. Thank you a lot. The code is below: def __str__(self):
if os.access(self.path,os.W_OK):
return u'The file is not writable. Permissions problem is detected ' + super(WriteError, self).text()
else:
return u'error writing ' + super(WriteError, self).text()

[sampsyo]

Hi! You can format the code using a single pair of ``` delimiters. See GitHub's Markdown help for details.

I don't quite understand what you're proposing here. The linked issue shows an exception being raised from a connection.execute call; we'll need to handle that exception. It looks like you might be looking at the code that's used to write music files themselves? But the issue at hand is about accessing the SQLite database.

[Mary011196]

Thank you very much. I will check it out again. 2017-03-30 6:15 GMT+03:00 Adrian Sampson <[email protected]>:

…

Hi! You can format the code using a single pair of ``` delimiters. See GitHub's Markdown help for details. I don't quite understand what you're proposing here. The linked issue shows an exception being raised from a connection.execute call; we'll need to handle that exception. It looks like you might be looking at the code that's used to write *music files* themselves? But the issue at hand is about accessing the SQLite database. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1676 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AOp3kvpu3hJ402TpGIXpqmjniVXf_iE-ks5rqx5lgaJpZM4GZQIU> .

[Mary011196]

I thought to add this in db.py. Is this right?

    def mutate(self, statement, subvals=()):
        """Execute an SQL statement with substitution values and return
        the row ID of the last affected row.
        """
        try:
            cursor = self.db._connection().execute(statement, subvals)
            return cursor.lastrowid
        except sqlite3.OperationalError,e:
            print ('Problem with database connection:%s' str(e))

Thank you

[sampsyo]

Thanks! This is indeed where the error occurs. There are a few more things to address:

• Just printing an error message won't cut it—we need to stop beets entirely if this happens. That usually happens by raising a UserError, but that's usually best done in UI-related code. So, we should probably catch this error in one of the "main" functions in beets.ui instead of here.
• An OperationalError can be raised in many different situations. We need to specifically detect when it indicates a permissions problem.
• For sensitive, central parts of the code like this, we really need tests to show that we've fixed the issue. Can you please start by crafting a unit test that triggers the problem so you can be sure that the code you add actually fixes it?
• The modern syntax for handling exceptions is except sqlite3.OperationalError as e:.

[Mary011196]

Hello,
Could you please help me? I change the permissions to my library but the command os.access(path,os.W_OK) returns true. Is there another way to check it?
I've made the unittest also.

Thank you in advance

[sampsyo]

Yes, checking the permissions ahead of time with os.stat and similar functions is probably not the right way to go. Instead, we'll want to let the SQLite module produce an exception, and then inspect that exception value to see what the explanation is.

As far as I can tell, there's no way to get a numerical error code out of an SQLite exception:
https://bugs.python.org/issue16379
https://bugs.python.org/issue24139

So the thing to do will be to use e.args[0] to get the error message and check whether that equals the error string referenced in the traceback above.

[Mary011196]

The e.args[0] gives the message unable to open the database file and the traceback too. So, how can i understand that this message indicates permission problem?

Thank you

[sampsyo]

That may be all the detail we can extract—in which case we'll just need to report that we were unable to open the database file, which might indicate a permissions problem.

[apitofme]

Was looking for a 'good first issue' ... this thread is old but with no apparent resolution so I went digging ... didn't take long for me to discover the following contained in ui/__init__.py:

def _open_library(config):
    """Create a new library instance from the configuration.
    """
    dbpath = util.bytestring_path(config['library'].as_filename())
    try:
        lib = library.Library(
            dbpath,
            config['directory'].as_filename(),
            get_path_formats(),
            get_replacements(),
        )
        lib.get_item(0)  # Test database connection.
    except (sqlite3.OperationalError, sqlite3.DatabaseError) as db_error:
        log.debug(u'{}', traceback.format_exc())
        raise UserError(u"database file {0} cannot not be opened: {1}".format(
            util.displayable_path(dbpath),
            db_error
        ))
    log.debug(u'library database: {0}\n'
              u'library directory: {1}',
              util.displayable_path(lib.path),
              util.displayable_path(lib.directory))
    return lib

...specifically the exception statement here suggests that the issue has been resolved?!

[sampsyo]

Hmm, good question. It would be worth investigating a little further to see how SQLite actually fails when the directory is read-only. From the original thread, for example, it seems possible that the error isn't raised until we actually try to write to the database—which would mean that this handler on the call to open the database wouldn't catch it.