This is an archive of the discontinued Mercurial Phabricator instance.

error: introduce StorageError
ClosedPublic

Authored by indygreg on Sep 18 2018, 8:06 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGcb65d4b7e429: error: introduce StorageError
Summary

Errors in revlogs are often represented by RevlogError. It's fine
for revlogs to raise a revlog-specific exception. But in the context
of multiple storage backends, it doesn't make sense to be throwing or
catching an exception with "revlog" in its name when revlogs may not
even be in play.

This commit introduces a new generic StorageError type for representing
errors in the storage layer.

RevlogError is an instance of this type.

Interface documentation and tests referencing RevlogError has been
updated to specify StorageError should be used.

.. api::

``error.StorageError`` has been introduced to represent errors in
storage. It should be used in place of ``error.RevlogError`` unless
the error is known to come from a revlog.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Sep 18 2018, 8:06 PM
yuja added a subscriber: yuja.Sep 23 2018, 6:11 AM

-class RevlogError(Hint, Exception):
+class StorageError(Hint, Exception):
+ """Raised when an error occurs in a storage layer.
+
+ Usually subclassed by a storage-specific exception.
+ """
+ bytes = _tobytes
+
+class RevlogError(StorageError):

__bytes__ = _tobytes

Perhaps LookupError will have to be either reparented to StorageError
or detached from the storage-type error. Just a note. I think you would have
some plan.

This revision was automatically updated to reflect the committed changes.
In D4654#71289, @yuja wrote:

-class RevlogError(Hint, Exception):
+class StorageError(Hint, Exception):
+ """Raised when an error occurs in a storage layer.
+
+ Usually subclassed by a storage-specific exception.
+ """
+ bytes = _tobytes
+
+class RevlogError(StorageError):

__bytes__ = _tobytes

Perhaps LookupError will have to be either reparented to StorageError
or detached from the storage-type error. Just a note. I think you would have
some plan.

I, uh, missed that LookupError inherited from RevlogError. I think it should inherit from StorageError directly.

Ideally, I think it shouldn't be part of StorageError. But I suspect a number of except StorageError will need updated to consider that. I'm not willing to scope bloat that work... yet.

I attempted the trivial reparenting to StorageError and this broke a test:

--- /home/gps/src/hg/tests/test-revset.t
+++ /home/gps/src/hg/tests/test-revset.t.err
@@ -1941,7 +1941,7 @@
   2:fffb6093b00943f91034b9bdad069402c834e572 fffb6
   3:fff48a9b9de34a4d64120c29548214c67980ade3 fff4
   4:ffff85cff0ff78504fcdc3c0bc10de0c65379249 ffff8
-  2147483647:ffffffffffffffffffffffffffffffffffffffff fffff
+  2147483647:ffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffff
   $ hg debugobsolete fffbae3886c8fbb2114296380d276fd37715d571
   obsoleted 1 changesets

Why, I'm not exactly sure. Perhaps an overzealous except RevlogError in revlog.py somewhere? I may track this down and fix it...

yuja added a comment.Sep 25 2018, 9:36 AM
    • /home/gps/src/hg/tests/test-revset.t +++ /home/gps/src/hg/tests/test-revset.t.err @@ -1941,7 +1941,7 @@ 2:fffb6093b00943f91034b9bdad069402c834e572 fffb6 3:fff48a9b9de34a4d64120c29548214c67980ade3 fff4 4:ffff85cff0ff78504fcdc3c0bc10de0c65379249 ffff8
  • 2147483647:ffffffffffffffffffffffffffffffffffffffff fffff + 2147483647:ffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffff $ hg debugobsolete fffbae3886c8fbb2114296380d276fd37715d571 obsoleted 1 changesets

It should be fixed by D4735.