Improve display of errors when transfers fail

Transfers from or to a local git repo could fail without a reason being
given, if the content failed to verify, or if the object file's stat
changed while it was being copied. Now display messages in these cases.

Sponsored-by: Jack Hill on Patreon

Annex/Content.hs

@@ -1,6 +1,6 @@
 {- git-annex file content managing
  -
- - Copyright 2010-2020 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2021 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -36,6 +36,7 @@ module Annex.Content (
 	linkOrCopy',
 	sendAnnex,
 	prepSendAnnex,
+	prepSendAnnex',
 	removeAnnex,
 	moveBad,
 	KeyLocation(..),
@@ -446,14 +447,15 @@ unlinkAnnex key = do
  - The rollback action should remove the data that was transferred.
  -}
 sendAnnex :: Key -> Annex () -> (FilePath -> Annex a) -> Annex a
-sendAnnex key rollback sendobject = go =<< prepSendAnnex key
+sendAnnex key rollback sendobject = go =<< prepSendAnnex' key
   where
-	go (Just (f, checksuccess)) = do
+	go (Just (f, check)) = do
 		r <- sendobject f
-		unlessM checksuccess $ do
+		check >>= \case
-			rollback
+			Nothing -> return r
-			giveup "content changed while it was being sent"
+			Just err -> do
-		return r
+				rollback
+				giveup err
 	go Nothing = giveup "content not available to send"
 
 {- Returns a file that contains an object's content,
@@ -483,6 +485,16 @@ prepSendAnnex key = withObjectLoc key $ \f -> do
 		then Nothing
 		else Just (fromRawFilePath f, sameInodeCache f cache')
 
+prepSendAnnex' :: Key -> Annex (Maybe (FilePath, Annex (Maybe String)))
+prepSendAnnex' key = prepSendAnnex key >>= \case
+	Just (f, checksuccess) -> 
+		let checksuccess' = ifM checksuccess
+			( return Nothing
+			, return (Just "content changed while it was being sent")
+			)
+		in return (Just (f, checksuccess'))
+	Nothing -> return Nothing
+
 cleanObjectLoc :: Key -> Annex () -> Annex ()
 cleanObjectLoc key cleaner = do
 	file <- calcRepo (gitAnnexLocation key)

Annex/CopyFile.hs

@@ -113,7 +113,9 @@ fileCopier copycowtried src dest k meterupdate check verifyconfig =
 			( case iv of
 				Just x -> ifM (liftIO $ finalizeIncremental x)
 					( return (True, Verified)
-					, return (False, UnVerified)
+					, do
+						warning "verification of content failed"
+						return (False, UnVerified)
 					)
 				Nothing -> return (True, UnVerified)
 			, return (False, UnVerified)

CHANGELOG

@@ -5,6 +5,7 @@ git-annex (8.20210622) UNRELEASED; urgency=medium
     synced/git-annex first.
   * Added annex.freezecontent-command and annex.thawcontent-command
     configs.
+  * Improve display of errors when transfers fail.
 
  -- Joey Hess <id@joeyh.name>  Mon, 21 Jun 2021 12:25:25 -0400
 

Remote/Git.hs

@@ -549,8 +549,11 @@ copyFromRemote'' repo forcersync r st@(State connpool _ _ _ _) key file dest met
 		u <- getUUID
 		hardlink <- wantHardLink
 		-- run copy from perspective of remote
-		onLocalFast st $ Annex.Content.prepSendAnnex key >>= \case
+		onLocalFast st $ Annex.Content.prepSendAnnex' key >>= \case
-			Just (object, checksuccess) -> do
+			Just (object, check) -> do
+				let checksuccess = check >>= \case
+					Just err -> giveup err
+					Nothing -> return True
 				let verify = Annex.Content.RemoteVerify r
 				copier <- mkFileCopier hardlink st
 				(ok, v) <- runTransfer (Transfer Download u (fromKey id key))
@@ -673,7 +676,7 @@ copyToRemote' :: Git.Repo -> Remote -> State -> Key -> AssociatedFile -> MeterUp
 copyToRemote' repo r st@(State connpool duc _ _ _) key file meterupdate
 	| not $ Git.repoIsUrl repo = ifM duc
 		( guardUsable repo (giveup "cannot access remote") $ commitOnCleanup repo r st $
-			copylocal =<< Annex.Content.prepSendAnnex key
+			copylocal =<< Annex.Content.prepSendAnnex' key
 		, giveup "remote does not have expected annex.uuid value"
 		)
 	| Git.repoIsSsh repo = commitOnCleanup repo r st $
@@ -684,11 +687,11 @@ copyToRemote' repo r st@(State connpool duc _ _ _) key file meterupdate
 	| otherwise = giveup "copying to non-ssh repo not supported"
   where
 	copylocal Nothing = giveup "content not available"
-	copylocal (Just (object, checksuccess)) = do
+	copylocal (Just (object, check)) = do
-		-- The checksuccess action is going to be run in
+		-- The check action is going to be run in
 		-- the remote's Annex, but it needs access to the local
 		-- Annex monad's state.
-		checksuccessio <- Annex.withCurrentState checksuccess
+		checkio <- Annex.withCurrentState check
 		u <- getUUID
 		hardlink <- wantHardLink
 		-- run copy from perspective of remote
@@ -698,9 +701,12 @@ copyToRemote' repo r st@(State connpool duc _ _ _) key file meterupdate
 				let verify = Annex.Content.RemoteVerify r
 				copier <- mkFileCopier hardlink st
 				let rsp = RetrievalAllKeysSecure
+				let checksuccess = liftIO checkio >>= \case
+					Just err -> giveup err
+					Nothing -> return True
 				res <- logStatusAfter key $ Annex.Content.getViaTmp rsp verify key file $ \dest ->
 					metered (Just (combineMeterUpdate meterupdate p)) key $ \_ p' -> 
-						copier object (fromRawFilePath dest) key p' (liftIO checksuccessio) verify
+						copier object (fromRawFilePath dest) key p' checksuccess verify
 				Annex.Content.saveState True
 				return res
 			)

doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_4_af8d80956d3ccedfe9aeb0fe22b2592a._comment

@@ -0,0 +1,27 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 4"""
+ date="2021-06-25T16:15:40Z"
+ content="""
+I've taken a close look at the error handling in the relevant code path.
+Normally when there's an exception it gets displayed. It is possible
+for the transfer to fail without displaying anything and without throwing an
+exception. When I synthesize such a failure, I see the same
+output you described.
+
+One way that could happen is if git-annex thinks the file got
+modified while it was being sent. If the file was unlocked, and was touched
+by something else while the copy was running, it would be enough to make
+git-annex think that, even though the content is still good. I've made
+it display a reason for the failure in that case.
+
+The only other way I found was if the hash fails to verify, which seems
+already ruled out in your case, but it did result in the same hard to understand
+output, so I've fixed that too.
+
+It would be good to know if one of these changes has fixed it for you.
+The autobuild for linux should be updated within an hour.
+While the scenario with the file having been touched seems unlikely to
+happen repeatedly with different files, it does otherwise match your
+circumstances and seems like the most likely fit.
+"""]]