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

diff --git a/Annex/Content.hs b/Annex/Content.hs
index 12af39618c0be59a69d2e359c51c55a9e26cb0e7..076fce1edb6758599a392baf88585d88a19cebd4 100644 (file)
--- a/Annex/Content.hs
+++ b/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
-                       rollback
-                       giveup "content changed while it was being sent"
-               return r
+               check >>= \case
+                       Nothing -> return r
+                       Just err -> do
+                               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)

diff --git a/Annex/CopyFile.hs b/Annex/CopyFile.hs
index b7d05cf97adf5e66544354bd2b1101a6e1438c69..63eb1a95f16f52009d55445a320c43421bd2cd6d 100644 (file)
--- a/Annex/CopyFile.hs
+++ b/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)

diff --git a/CHANGELOG b/CHANGELOG
index 9c32f4ad8987bb00bfbb7aabebfd4381a368beac..69cab38a45ee1ee9812577546e784b26316f5ca1 100644 (file)
--- a/CHANGELOG
+++ b/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
 

diff --git a/Remote/Git.hs b/Remote/Git.hs
index 43105a4fcd5dd3fa6ec5ba0237f120b6270c85b9..c2a23789a5b664850f92a16491c141d41c7dbaac 100644 (file)
--- a/Remote/Git.hs
+++ b/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
-                       Just (object, checksuccess) -> do
+               onLocalFast st $ Annex.Content.prepSendAnnex' key >>= \case
+                       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
-               -- The checksuccess action is going to be run in
+       copylocal (Just (object, check)) = do
+               -- 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
                        )

diff --git a/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_4_af8d80956d3ccedfe9aeb0fe22b2592a._comment b/doc/bugs/__34__failed_to_send_content_to_remote__34__/comment_4_af8d80956d3ccedfe9aeb0fe22b2592a._comment
new file mode 100644 (file)
index 0000000..3af04e4
--- /dev/null
+++ b/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.
+"""]]