This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4654

error: introduce StorageError
ClosedPublic

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

Download Raw Diff

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

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

indygreg created this revision.Sep 18 2018, 8:06 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptSep 18 2018, 8:06 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

indygreg added a child revision: D4655: global: replace most uses of RevlogError with StorageError (API).Sep 18 2018, 8:06 PM

-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.

Closed by commit rHGcb65d4b7e429: error: introduce StorageError (authored by indygreg). · Explain WhySep 23 2018, 6:13 AM

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...

/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.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/error.py (9 lines)
M			mercurial/repository.py (2 lines)
M			mercurial/testing/storage.py (20 lines)

Commit	Parents	Author	Summary	Date
		Gregory Szorc		Sep 18 2018, 7:45 PM

Status	Author	Revision
Closed	indygreg	D4663 filelog: stop proxying headrevs() (API)
Closed	indygreg	D4662 hgweb: use heads() instead of headrevs()
Closed	indygreg	D4661 filelog: record what's using attributes
Closed	indygreg	D4660 filelog: stop proxying datafile (API)
Closed	indygreg	D4659 filelog: stop proxying _addrevision() (API)
Closed	indygreg	D4658 filelog: stop proxying compress() (API)
Closed	indygreg	D4657 filelog: stop proxying start(), end(), and length() (API)
Closed	indygreg	D4656 revlog: move censor logic out of censor extension
Closed	indygreg	D4655 global: replace most uses of RevlogError with StorageError (API)
Closed	indygreg	D4654 error: introduce StorageError
Closed	indygreg	D4653 revlog: drop LookupError alias (API)
Closed	indygreg	D4652 revlog: drop some more error aliases (API)
Closed	indygreg	D4651 revlog: drop RevlogError alias (API)
Closed	indygreg	D4650 testing: add interface unit tests for file storage

Diff 11188

mercurial/error.py


	This should come first in the inheritance list to consume a hint and			This should come first in the inheritance list to consume a hint and
	pass remaining arguments to the exception class.			pass remaining arguments to the exception class.
	"""			"""
	def __init__(self, args, *kw):			def __init__(self, args, *kw):
	self.hint = kw.pop(r'hint', None)			self.hint = kw.pop(r'hint', None)
	super(Hint, self).__init__(args, *kw)			super(Hint, self).__init__(args, *kw)

	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			__bytes__ = _tobytes

	class FilteredIndexError(IndexError):			class FilteredIndexError(IndexError):
	__bytes__ = _tobytes			__bytes__ = _tobytes

	class LookupError(RevlogError, KeyError):			class LookupError(RevlogError, KeyError):
	def __init__(self, name, index, message):			def __init__(self, name, index, message):
	self.name = name			self.name = name

mercurial/repository.py


	Any metadata is excluded from size measurements. Use ``rawsize()`` if			Any metadata is excluded from size measurements. Use ``rawsize()`` if
	metadata size is important.			metadata size is important.
	"""			"""

	def checkhash(fulltext, node, p1=None, p2=None, rev=None):			def checkhash(fulltext, node, p1=None, p2=None, rev=None):
	"""Validate the stored hash of a given fulltext and node.			"""Validate the stored hash of a given fulltext and node.

	Raises ``error.RevlogError`` is hash validation fails.			Raises ``error.StorageError`` is hash validation fails.
	"""			"""

	def revision(node, raw=False):			def revision(node, raw=False):
	""""Obtain fulltext data for a node.			""""Obtain fulltext data for a node.

	By default, any storage transformations are applied before the data			By default, any storage transformations are applied before the data
	is returned. If ``raw`` is True, non-raw storage transformations			is returned. If ``raw`` is True, non-raw storage transformations
	are not applied.			are not applied.

mercurial/testing/storage.py


	for i in range(-5, 5):			for i in range(-5, 5):
	if i == nullrev:			if i == nullrev:
	continue			continue

	with self.assertRaises(IndexError):			with self.assertRaises(IndexError):
	f.size(i)			f.size(i)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(b'', nullid)			f.checkhash(b'', nullid)

	with self.assertRaises(error.LookupError):			with self.assertRaises(error.LookupError):
	f.checkhash(b'', b'\x01' * 20)			f.checkhash(b'', b'\x01' * 20)

	self.assertEqual(f.revision(nullid), b'')			self.assertEqual(f.revision(nullid), b'')
	self.assertEqual(f.revision(nullid, raw=True), b'')			self.assertEqual(f.revision(nullid, raw=True), b'')

	self.assertEqual(f.size(0), len(fulltext))			self.assertEqual(f.size(0), len(fulltext))

	with self.assertRaises(IndexError):			with self.assertRaises(IndexError):
	f.size(1)			f.size(1)

	f.checkhash(fulltext, node)			f.checkhash(fulltext, node)
	f.checkhash(fulltext, node, nullid, nullid)			f.checkhash(fulltext, node, nullid, nullid)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(fulltext + b'extra', node)			f.checkhash(fulltext + b'extra', node)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(fulltext, node, b'\x01' * 20, nullid)			f.checkhash(fulltext, node, b'\x01' * 20, nullid)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(fulltext, node, nullid, b'\x01' * 20)			f.checkhash(fulltext, node, nullid, b'\x01' * 20)

	self.assertEqual(f.revision(node), fulltext)			self.assertEqual(f.revision(node), fulltext)
	self.assertEqual(f.revision(node, raw=True), fulltext)			self.assertEqual(f.revision(node, raw=True), fulltext)

	self.assertEqual(f.read(node), fulltext)			self.assertEqual(f.read(node), fulltext)

	self.assertFalse(f.renamed(node))			self.assertFalse(f.renamed(node))
	with self.assertRaises(IndexError):			with self.assertRaises(IndexError):
	f.size(3)			f.size(3)

	f.checkhash(fulltext0, node0)			f.checkhash(fulltext0, node0)
	f.checkhash(fulltext1, node1)			f.checkhash(fulltext1, node1)
	f.checkhash(fulltext1, node1, node0, nullid)			f.checkhash(fulltext1, node1, node0, nullid)
	f.checkhash(fulltext2, node2, node1, nullid)			f.checkhash(fulltext2, node2, node1, nullid)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(fulltext1, b'\x01' * 20)			f.checkhash(fulltext1, b'\x01' * 20)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(fulltext1 + b'extra', node1, node0, nullid)			f.checkhash(fulltext1 + b'extra', node1, node0, nullid)

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.checkhash(fulltext1, node1, node0, node0)			f.checkhash(fulltext1, node1, node0, node0)

	self.assertEqual(f.revision(node0), fulltext0)			self.assertEqual(f.revision(node0), fulltext0)
	self.assertEqual(f.revision(node0, raw=True), fulltext0)			self.assertEqual(f.revision(node0, raw=True), fulltext0)
	self.assertEqual(f.revision(node1), fulltext1)			self.assertEqual(f.revision(node1), fulltext1)
	self.assertEqual(f.revision(node1, raw=True), fulltext1)			self.assertEqual(f.revision(node1, raw=True), fulltext1)
	self.assertEqual(f.revision(node2), fulltext2)			self.assertEqual(f.revision(node2), fulltext2)
	self.assertEqual(f.revision(node2, raw=True), fulltext2)			self.assertEqual(f.revision(node2, raw=True), fulltext2)
	self.assertEqual(node1, node0)			self.assertEqual(node1, node0)
	self.assertEqual(node2, node0)			self.assertEqual(node2, node0)
	self.assertEqual(len(f), 1)			self.assertEqual(len(f), 1)

	def testaddrevisionbadnode(self):			def testaddrevisionbadnode(self):
	f = self._makefilefn()			f = self._makefilefn()
	with self._maketransactionfn() as tr:			with self._maketransactionfn() as tr:
	# Adding a revision with bad node value fails.			# Adding a revision with bad node value fails.
	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.addrevision(b'foo', tr, 0, nullid, nullid, node=b'\x01' * 20)			f.addrevision(b'foo', tr, 0, nullid, nullid, node=b'\x01' * 20)

	def testaddrevisionunknownflag(self):			def testaddrevisionunknownflag(self):
	f = self._makefilefn()			f = self._makefilefn()
	with self._maketransactionfn() as tr:			with self._maketransactionfn() as tr:
	for i in range(15, 0, -1):			for i in range(15, 0, -1):
	if (1 << i) & ~revlog.REVIDX_KNOWN_FLAGS:			if (1 << i) & ~revlog.REVIDX_KNOWN_FLAGS:
	flags = 1 << i			flags = 1 << i
	break			break

	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.addrevision(b'foo', tr, 0, nullid, nullid, flags=flags)			f.addrevision(b'foo', tr, 0, nullid, nullid, flags=flags)

	def testaddgroupsimple(self):			def testaddgroupsimple(self):
	f = self._makefilefn()			f = self._makefilefn()

	callbackargs = []			callbackargs = []
	def cb(args, *kwargs):			def cb(args, *kwargs):
	callbackargs.append((args, kwargs))			callbackargs.append((args, kwargs))
	fulltext0 = b'foo'			fulltext0 = b'foo'
	delta0 = mdiff.trivialdiffheader(len(fulltext0)) + fulltext0			delta0 = mdiff.trivialdiffheader(len(fulltext0)) + fulltext0

	deltas = [			deltas = [
	(b'\x01' * 20, nullid, nullid, nullid, nullid, delta0, 0),			(b'\x01' * 20, nullid, nullid, nullid, nullid, delta0, 0),
	]			]

	with self._maketransactionfn() as tr:			with self._maketransactionfn() as tr:
	with self.assertRaises(error.RevlogError):			with self.assertRaises(error.StorageError):
	f.addgroup(deltas, linkmapper, tr, addrevisioncb=cb)			f.addgroup(deltas, linkmapper, tr, addrevisioncb=cb)

	node0 = f.add(fulltext0, None, tr, 0, nullid, nullid)			node0 = f.add(fulltext0, None, tr, 0, nullid, nullid)

	f = self._makefilefn()			f = self._makefilefn()

	deltas = [			deltas = [
	(node0, nullid, nullid, nullid, nullid, delta0, 0),			(node0, nullid, nullid, nullid, nullid, delta0, 0),

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	11188		Sep 18 2018, 8:06 PM	★	★
Diff 2	11280	rHGcb65d4b7e429b1ab9824cfaafc85a220b00760a3	Sep 18 2018, 7:45 PM	★	★