Blair Zajac wrote on Tue, Dec 21, 2010 at 10:55:37 -0800: > On 12/21/10 10:40 AM, Daniel Shahaf wrote: >> Blair Zajac wrote on Tue, Dec 21, 2010 at 10:16:56 -0800: >>> 4) In svn_repos_fs_commit_txn(), which order should errors be composed? >>> svn_fs_commit_txn()'s error as the parent followed by the >>> SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED error as a child? This seems to be >>> the standard ordering of chained errors. On the other hand, it makes it >>> harder to find a post-commit script error. >> >> Actually, it will make it impossible to detect post-commit errors over >> ra_dav, since that RA layer marshals only the outermost error code in an >> error chain. > > ra_dav already checks for SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED, it could > use svn_error_has_cause() to find it. If ra_dav cannot find a > SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED but there is an error and the > commit succeeded, then I think the parent error should be returned to > warn people there may be an issue with the server.
(I'm reading s/ra_dav/mod_dav_svn/) +1, modulo one change: the "there may be issue with the server" is not just when SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED isn't found, but also when that is found but svn_fs_commit_txn() failed as well. I'm not sure I have a good suggestion, but principly we should be sure we allow the user/admin to understand exactly what happened --- post-commit hook failed, post-commit FS juggling failed, or both. > Blair
