fixup error handling if mononoke fetches fail

Summary:
We were catching exceptions that occurred during setup of
the mononoke futures rather than catching errors that occurred during
processing of the mononoke futures themselves.

This diff switches the try/catch blocks to `onError` handlers on
the future chains, as well as improves an ambiguous error message.

Reviewed By: chadaustin

Differential Revision: D10297428

fbshipit-source-id: c87e4a23a70e010646b07d9d8728851bdfdbcc2a
This commit is contained in:
Wez Furlong 2018-10-10 14:44:45 -07:00 committed by Facebook Github Bot
parent fffabfe6a0
commit 7643da4464
2 changed files with 49 additions and 35 deletions

View File

@ -324,9 +324,8 @@ Future<unique_ptr<Tree>> HgBackingStore::importTreeImpl(
RelativePath ownedPath(path);
return mononoke_->getTree(manifestNode)
.within(std::chrono::milliseconds(FLAGS_mononoke_timeout))
.thenTry([this, manifestNode, edenTreeID, ownedPath, writeBatch](
.thenTry([edenTreeID, ownedPath, writeBatch](
auto mononokeTreeTry) mutable {
try {
auto& mononokeTree = mononokeTreeTry.value();
std::vector<TreeEntry> entries;
@ -345,7 +344,9 @@ Future<unique_ptr<Tree>> HgBackingStore::importTreeImpl(
writeBatch->put(
KeySpace::TreeFamily, edenTreeID, serialized.second.coalesce());
return makeFuture(std::move(tree));
} catch (const std::exception& ex) {
})
.onError([this, manifestNode, edenTreeID, ownedPath, writeBatch](
const folly::exception_wrapper& ex) mutable {
XLOG(WARN) << "got exception from MononokeBackingStore: "
<< ex.what();
return fetchTreeFromImporter(
@ -353,7 +354,6 @@ Future<unique_ptr<Tree>> HgBackingStore::importTreeImpl(
edenTreeID,
std::move(ownedPath),
std::move(writeBatch));
}
});
}
#endif // !EDEN_WIN_NOMONONOKE
@ -544,13 +544,23 @@ Future<unique_ptr<Blob>> HgBackingStore::getBlob(const Hash& id) {
if (mononoke_) {
XLOG(DBG5) << "requesting file contents of '" << hgInfo.path() << "', "
<< hgInfo.revHash().toString() << " from mononoke";
try {
return mononoke_->getBlob(hgInfo.revHash());
} catch (const std::exception& ex) {
XLOG(WARN) << "Error while fetching file contents of '" << hgInfo.path()
<< "', " << hgInfo.revHash().toString()
<< " from mononoke: " << ex.what();
}
auto revHashCopy = hgInfo.revHash();
return mononoke_->getBlob(revHashCopy)
.onError([this, id, path = hgInfo.path(), revHash = revHashCopy](
const folly::exception_wrapper& ex) {
XLOG(ERR) << "Error while fetching file contents of '" << path
<< "', " << revHash.toString()
<< " from mononoke: " << ex.what()
<< ", fall back to import helper.";
return folly::via(
importThreadPool_.get(),
[id] {
return getThreadLocalImporter().importFileContents(id);
})
// Ensure that the control moves back to the main thread pool
// to process the caller-attached .then routine.
.via(serverThreadPool_);
});
}
#endif // EDEN_WIN_NOMONONOKE

View File

@ -251,6 +251,10 @@ folly::Future<std::unique_ptr<Tree>> MononokeBackingStore::getTreeForCommit(
});
}
namespace {
const std::string kTierName{"mononoke-apiserver"};
}
folly::Future<folly::SocketAddress> MononokeBackingStore::getAddress(
folly::EventBase* eventBase) {
if (socketAddress_.hasValue()) {
@ -263,7 +267,7 @@ folly::Future<folly::SocketAddress> MononokeBackingStore::getAddress(
auto selector = factory.getSelector();
selector->getSelectionAsync(
"mononoke-apiserver",
kTierName,
servicerouter::DebugContext(),
servicerouter::SelectionCacheCallback(
[promise = std::move(promise)](
@ -271,7 +275,7 @@ folly::Future<folly::SocketAddress> MononokeBackingStore::getAddress(
servicerouter::DebugContext&& /* unused */) mutable {
if (selection.hosts.empty()) {
auto ex = make_exception_wrapper<std::runtime_error>(
std::string("no host found"));
folly::to<std::string>("no hosts found in tier ", kTierName));
promise.setException(ex);
return;
}