Skip to content

Better error message when directory containing database file is not writeable #1676

@sampsyo

Description

@sampsyo
Member

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.

Activity

added a commit that references this issue on Jan 5, 2016
Mary011196

Mary011196 commented on Mar 29, 2017

@Mary011196
Contributor

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

sampsyo commented on Mar 30, 2017

@sampsyo
MemberAuthor

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

Mary011196 commented on Mar 30, 2017

@Mary011196
Contributor
Mary011196

Mary011196 commented on Mar 30, 2017

@Mary011196
Contributor

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

sampsyo commented on Mar 30, 2017

@sampsyo
MemberAuthor

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

Mary011196 commented on Apr 8, 2017

@Mary011196
Contributor

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

sampsyo commented on Apr 9, 2017

@sampsyo
MemberAuthor

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

Mary011196 commented on Apr 10, 2017

@Mary011196
Contributor

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

sampsyo commented on Apr 10, 2017

@sampsyo
MemberAuthor

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

apitofme commented on Aug 17, 2019

@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

sampsyo commented on Aug 17, 2019

@sampsyo
MemberAuthor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugbugs that are confirmed and actionablegood first issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @sampsyo@arcresu@apitofme@Mary011196@jtpavlock

      Issue actions

        Better error message when directory containing database file is not writeable · Issue #1676 · beetbox/beets