Closed Bug 1423729 Opened 6 years ago Closed 8 months ago

Check for connection open error reason in FormHistory.jsm once bug 1407778 lands

Categories

(Toolkit :: Form Manager, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
118 Branch
Tracking Status
firefox118 --- fixed

People

(Reporter: mconley, Assigned: joschmidt)

References

Details

(Whiteboard: [fxcm-tech-debt])

Attachments

(1 file)

Once bug 1407778 lands, opening connections to SQLite databases with Sqlite.jsm can give us more fine-grained reasons for why they fail (example: out of disk space, or locked by anti-virus).

Bug 888784 will land support for using Sqlite.jsm in FormHistory.jsm. That'll likely land before bug 1407778 lands.

Once bug 1407778 lands, we should go back, and modify FormHistory.jsm to inspect the reason why the async database connection fails. If it fails because we're out of disk space or locked by anti-virus, we should probably bail out of trying to connect to the database completely rather than trying again.
Depends on: 888784
P3?
Priority: -- → P3
Severity: normal → S3

Not sure if we can detect "locked by anti-virus", but SQLite is known to give "blocked" failures when database is locked for write. We could inspect result and attempt to reconnect 3 times after 50, 100, and 200ms for example.

Johannes, what do you think?

Flags: needinfo?(joschmidt)

After reading into the code a bit, I couldn't agree more.
Currently, we handle errors during SQLite connection establishment non-specifically. There is a retry mechanism that does the following N -times:

  1. close the connection if open
  2. create a backup
  3. delete the current database file
  4. retry

where n is 2. This means that the database is reset every time a database error occurs during the establishment of the connection. This can hardly be the expected behavior, especially in cases where the error is potentially temporary. (Also, it doesn't seem to make much sense to try to create a backup when memory is full, but that's just as an aside).

Instead, depending on the error, we can do three things:

  1. retry N times immediately with reset (current situation). For cases where it is a database file problem.
  2. retry with exponential backoff, without resetting the database. For external problems, which may be fixed after some time
  3. give up directly if there is no hope

To get closer to this, let's first have a look at the possible errors. At https://searchfox.org/mozilla-central/source/toolkit/modules/Sqlite.sys.mjs#134-172 you can find a list of all errors and their assignment to mozIStorageError. These errors can be grouped into three different categories depending on their treatment:

Database File Errors

Errors on the file can only be fixed by resetting the database:

  • file corrupted
  • too large
  • unexpected
Suspected Temporary Errors

In the case of an error that will probably be fixed in the short term, a retry is worthwhile:

  • abort
  • locked
  • busy
  • restriction
  • memory io error
Potentially Permanent System Errors

If these are fundamental errors, which are likely to be of longer duration, there is nothing to do. (For simplicity, however, these errors can be handled by retry).

  • access denied
  • write protected
  • no memory

What do you think?

Flags: needinfo?(joschmidt)
Assignee: nobody → joschmidt
Whiteboard: [fxcm-tech-debt]

Confirmed that in case of an ACCESS_DENIED error, the database is reset.

Attachment #9345739 - Attachment description: WIP: - Bug 1423729 - Add test for access denied db reset → WIP: - Bug 1423729 - handle form history database establishment failures - r?#credential-management-reviewers!
Attachment #9345739 - Attachment description: WIP: - Bug 1423729 - handle form history database establishment failures - r?#credential-management-reviewers! → Bug 1423729 - improve form history database establishment failure handling - r?#credential-management-reviewers!
Pushed by joschmidt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc65f490e5cf
improve form history database establishment failure handling - r=credential-management-reviewers,issammani
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
You need to log in before you can comment on or make changes to this bug.