Improved error handling for cases where blob account credentials are either not found in the provided credentials sources and/or some of the credentials sources provided are not readable or parseable.

fdbrpc/BlobStore.actor.cpp

@@ -221,21 +221,27 @@ ACTOR Future<Void> deleteRecursively_impl(Reference<BlobStoreEndpoint> b, std::s
 	state Future<Void> done = b->listBucketStream(bucket, resultStream, prefix, '/', std::numeric_limits<int>::max());
 	// Wrap done in an actor which will send end_of_stream since listBucketStream() does not (so that many calls can write to the same stream)
 	done = map(done, [=](Void) {
-			resultStream.sendError(end_of_stream());
+		resultStream.sendError(end_of_stream());
-			return Void();
+		return Void();
-		});
+	});
 
 	state std::list<Future<Void>> deleteFutures;
 	try {
 		loop {
-			BlobStoreEndpoint::ListResult list = waitNext(resultStream.getFuture());
+			choose {
-			for(auto &object : list.objects) {
+				// Throw if done throws, otherwise don't stop until end_of_stream
-				int *pNumDeletedCopy = pNumDeleted;   // avoid capture of this
+				when(Void _ = wait(done)) {}
-				deleteFutures.push_back(map(b->deleteObject(bucket, object.name), [pNumDeletedCopy](Void) -> Void {
+
-					if(pNumDeletedCopy != nullptr)
+				when(BlobStoreEndpoint::ListResult list = waitNext(resultStream.getFuture())) {
-						++*pNumDeletedCopy;
+					for(auto &object : list.objects) {
-					return Void();
+						int *pNumDeletedCopy = pNumDeleted;   // avoid capture of this
-				}));
+						deleteFutures.push_back(map(b->deleteObject(bucket, object.name), [pNumDeletedCopy](Void) -> Void {
+							if(pNumDeletedCopy != nullptr)
+								++*pNumDeletedCopy;
+							return Void();
+						}));
+					}
+				}
 			}
 
 			// This is just a precaution to avoid having too many outstanding delete actors waiting to run
@@ -290,9 +296,12 @@ Future<int64_t> BlobStoreEndpoint::objectSize(std::string const &bucket, std::st
 // Try to read a file, parse it as JSON, and return the resulting document.
 // It will NOT throw if any errors are encountered, it will just return an empty
 // JSON object and will log trace events for the errors encountered.
-ACTOR Future<json_spirit::mObject> tryReadJSONFile(std::string path) {
+ACTOR Future<Optional<json_spirit::mObject>> tryReadJSONFile(std::string path) {
 	state std::string content;
 
+	// Event type to be logged in the event of an exception
+	state const char *errorEventType = "BlobCredentialFileError";
+
 	try {
 		state Reference<IAsyncFile> f = wait(IAsyncFileSystem::filesystem()->open(path, IAsyncFile::OPEN_NO_AIO | IAsyncFile::OPEN_READONLY | IAsyncFile::OPEN_UNCACHED, 0));
 		state int64_t size = wait(f->size());
@@ -300,25 +309,22 @@ ACTOR Future<json_spirit::mObject> tryReadJSONFile(std::string path) {
 		int r = wait(f->read(mutateString(buf), size, 0));
 		ASSERT(r == size);
 		content = buf.toString();
-	} catch(Error &e) {
-		if(e.code() != error_code_actor_cancelled)
-			TraceEvent(SevWarn, "BlobCredentialFileError").detail("File", path).error(e).suppressFor(60, true);
-		return json_spirit::mObject();
-	}
 
-	try {
+		// Any exceptions from hehre forward are parse failures
+		errorEventType = "BlobCredentialFileParseFailed";
 		json_spirit::mValue json;
 		json_spirit::read_string(content, json);
 		if(json.type() == json_spirit::obj_type)
 			return json.get_obj();
 		else
 			TraceEvent(SevWarn, "BlobCredentialFileNotJSONObject").detail("File", path).suppressFor(60, true);
+
 	} catch(Error &e) {
 		if(e.code() != error_code_actor_cancelled)
-			TraceEvent(SevWarn, "BlobCredentialFileParseFailed").detail("File", path).error(e).suppressFor(60, true);
+			TraceEvent(SevWarn, errorEventType).detail("File", path).error(e).suppressFor(60, true);
 	}
 
-	return json_spirit::mObject();
+	return Optional<json_spirit::mObject>();
 }
 
 ACTOR Future<Void> updateSecret_impl(Reference<BlobStoreEndpoint> b) {
@@ -326,7 +332,7 @@ ACTOR Future<Void> updateSecret_impl(Reference<BlobStoreEndpoint> b) {
 	if(pFiles == nullptr)
 		return Void();
 
-	state std::vector<Future<json_spirit::mObject>> reads;
+	state std::vector<Future<Optional<json_spirit::mObject>>> reads;
 	for(auto &f : *pFiles)
 		reads.push_back(tryReadJSONFile(f));
 
@@ -334,13 +340,22 @@ ACTOR Future<Void> updateSecret_impl(Reference<BlobStoreEndpoint> b) {
 
 	std::string key = b->key + "@" + b->host;
 
+	int invalid = 0;
+
 	for(auto &f : reads) {
-		JSONDoc doc(f.get());
+		// If value not present then the credentials file wasn't readable or valid.  Continue to check other results.
+		if(!f.get().present()) {
+			++invalid;
+			continue;
+		}
+
+		JSONDoc doc(f.get().get());
 		if(doc.has("accounts") && doc.last().type() == json_spirit::obj_type) {
 			JSONDoc accounts(doc.last().get_obj());
 			if(accounts.has(key, false) && accounts.last().type() == json_spirit::obj_type) {
 				JSONDoc account(accounts.last());
 				std::string secret;
+				// Once we find a matching account, use it.
 				if(account.tryGet("secret", secret)) {
 					b->secret = secret;
 					return Void();
@@ -349,7 +364,12 @@ ACTOR Future<Void> updateSecret_impl(Reference<BlobStoreEndpoint> b) {
 		}
 	}
 
-	return Void();
+	// If any sources were invalid
+	if(invalid > 0)
+		throw backup_auth_unreadable();
+
+	// All sources were valid but didn't contain the desired info
+	throw backup_auth_missing();
 }
 
 Future<Void> BlobStoreEndpoint::updateSecret() {
@@ -451,12 +471,9 @@ ACTOR Future<Reference<HTTP::Response>> doRequest_impl(Reference<BlobStoreEndpoi
 			rconn.conn.clear();
 
 		} catch(Error &e) {
-			// For timeouts, conn failure, or bad reponse reported by HTTP:doRequest, save the error and handle it / possibly retry below.
+			if(e.code() == error_code_actor_cancelled)
-			// Any other error is rethrown.
-			if(e.code() == error_code_connection_failed || e.code() == error_code_timed_out || e.code() == error_code_http_bad_response)
-				err = e;
-			else
 				throw;
+			err = e;
 		}
 
 		// If err is not present then r is valid.
@@ -667,15 +684,21 @@ ACTOR Future<BlobStoreEndpoint::ListResult> listBucket_impl(Reference<BlobStoreE
 	state Future<Void> done = bstore->listBucketStream(bucket, resultStream, prefix, delimiter, maxDepth, recurseFilter);
 	// Wrap done in an actor which sends end_of_stream because list does not so that many lists can write to the same stream
 	done = map(done, [=](Void) {
-			resultStream.sendError(end_of_stream());
+		resultStream.sendError(end_of_stream());
-			return Void();
+		return Void();
-		});
+	});
 
 	try {
 		loop {
-			BlobStoreEndpoint::ListResult info = waitNext(resultStream.getFuture());
+			choose {
-			results.commonPrefixes.insert(results.commonPrefixes.end(), info.commonPrefixes.begin(), info.commonPrefixes.end());
+				// Throw if done throws, otherwise don't stop until end_of_stream
-			results.objects.insert(results.objects.end(), info.objects.begin(), info.objects.end());
+				when(Void _ = wait(done)) {}
+
+				when(BlobStoreEndpoint::ListResult info = waitNext(resultStream.getFuture())) {
+					results.commonPrefixes.insert(results.commonPrefixes.end(), info.commonPrefixes.begin(), info.commonPrefixes.end());
+					results.objects.insert(results.objects.end(), info.objects.begin(), info.objects.end());
+				}
+			}
 		}
 	} catch(Error &e) {
 		if(e.code() != error_code_end_of_stream)

flow/error_definitions.h

@@ -174,6 +174,8 @@ ERROR( backup_bad_block_size, 2313, "Backup file block size too small")
 ERROR( backup_invalid_url, 2314, "Backup Container URL invalid")
 ERROR( backup_invalid_info, 2315, "Backup Container URL invalid")
 ERROR( backup_cannot_expire, 2316, "Cannot expire requested data from backup without violating minimum restorability")
+ERROR( backup_auth_missing, 2317, "Cannot find authentication details (such as a password or secret key) for the specified Backup Container URL")
+ERROR( backup_auth_unreadable, 2318, "Cannot read or parse one or more sources of authentication information for Backup Container URLs")
 ERROR( restore_invalid_version, 2361, "Invalid restore version")
 ERROR( restore_corrupted_data, 2362, "Corrupted backup data")
 ERROR( restore_missing_data, 2363, "Missing backup data")