Skip to content

Commit

Permalink
[WIP] Introduce a Error exception to handle error
Browse files Browse the repository at this point in the history 
- To be split in several commit as we also add a argument to selectBooks
- We should better build the response message by composing a generic
  error response with the error exception.
  • Loading branch information
@mgautierfr
mgautierfr committed Mar 30, 2022
1 parent 8637a8e commit d72ea4c
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 45 deletions.
121 changes: 81 additions & 40 deletions src/server/internalServer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,33 @@ namespace kiwix {
namespace
{


enum class ErrorKind {
InvalidRequest,
BookNotFound,
ArchiveNotFound,
UrlNotFound
};

class Error: public std::exception {
public:
Error(ErrorKind kind, const std::string& detail)
: m_kind(kind),
detail(detail)
{}
virtual ~Error() = default;

virtual const char* what() const noexcept {
return detail.c_str();
}
ErrorKind kind() const noexcept {
return m_kind;
}
protected:
ErrorKind m_kind;
std::string detail;
};

inline std::string normalizeRootUrl(std::string rootUrl)
{
while ( !rootUrl.empty() && rootUrl.back() == '/' )
Expand Down Expand Up @@ -145,15 +172,15 @@ std::vector<T> subrange(const std::vector<T>& v, size_t s, size_t n)

} // unnamed namespace

std::set<std::string> InternalServer::selectBooks(const RequestContext& request, const std::string& prefix) const
std::set<std::string> InternalServer::selectBooks(const RequestContext& request, const std::string& prefix, bool mandatory) const
{
// Try old API
try {
auto bookName = request.get_argument("content");
try {
return {mp_nameMapper->getIdForName(bookName)};
} catch (const std::out_of_range&) {
throw std::invalid_argument("The requested book doesn't exist.");
throw Error(ErrorKind::BookNotFound, bookName);
}
} catch(const std::out_of_range&) {
// We've catch the out_of_range of get_argument
Expand All @@ -174,13 +201,16 @@ std::set<std::string> InternalServer::selectBooks(const RequestContext& request,
try {
bookIds.insert(mp_nameMapper->getIdForName(bookName));
} catch(const std::out_of_range&) {
throw std::invalid_argument("The requested book doesn't exist.");
throw Error(ErrorKind::BookNotFound, bookName);
}
}
return bookIds;
} catch(const std::out_of_range&) {}

// Check for filtering
if (mandatory) {
throw Error(ErrorKind::InvalidRequest, "No book query.");
}
Filter filter = get_search_filter(request, prefix+"query.");
auto id_vec = mp_library->filter(filter);
return std::set<std::string>(id_vec.begin(), id_vec.end());
Expand All @@ -196,9 +226,10 @@ SearchInfo::SearchInfo(const std::string& pattern, GeoQuery geoQuery)
geoQuery(geoQuery)
{}

SearchInfo::SearchInfo(const RequestContext& request)
SearchInfo::SearchInfo(const RequestContext& request, const std::set<std::string>& bookIds)
: pattern(request.get_optional_param<std::string>("pattern", "")),
geoQuery()
geoQuery(),
bookIds(bookIds)
{
/* Retrive geo search */
try {
Expand All @@ -210,13 +241,9 @@ SearchInfo::SearchInfo(const RequestContext& request)
catch(const std::invalid_argument&) {}

if (!geoQuery && pattern.empty()) {
throw std::invalid_argument("No query provided.");
throw Error(ErrorKind::InvalidRequest, "No query provided.");
}

try {
auto content_vector = request.get_arguments("content");
bookNames = std::set<std::string>(content_vector.begin(), content_vector.end());
} catch (const std::out_of_range&) {}
}

zim::Query SearchInfo::getZimQuery(bool verbose) const {
Expand Down Expand Up @@ -405,7 +432,7 @@ MHD_Result InternalServer::handlerCallback(struct MHD_Connection* connection,

std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& request)
{
try {
// try {
if (! request.is_valid_url()) {
return HTTP404HtmlResponse(*this, request)
+ urlNotFoundMsg;
Expand Down Expand Up @@ -437,13 +464,13 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
return handle_captured_external(request);

return handle_content(request);
} catch (std::exception& e) {
/* } catch (std::exception& e) {
fprintf(stderr, "===== Unhandled error : %s\n", e.what());
return Response::build_500(*this, e.what());
} catch (...) {
fprintf(stderr, "===== Unhandled unknown error\n");
return Response::build_500(*this, "Unknown error");
}
}*/
}

MustacheData InternalServer::get_default_data() const
Expand Down Expand Up @@ -536,16 +563,26 @@ std::unique_ptr<Response> InternalServer::handle_suggest(const RequestContext& r
std::string bookId;
std::shared_ptr<zim::Archive> archive;
try {
bookId = *selectBooks(request, "").begin();
bookId = *selectBooks(request, "", true).begin();
try {
archive = mp_library->getArchiveById(bookId);
} catch (const std::out_of_range&) {
throw std::invalid_argument("The requested archive doesn't exist.")
throw Error(ErrorKind::ArchiveNotFound, bookId);
}
} catch(const std::invalid_argument& e) {
} catch(const Error& e) {
// We need the bookName to build the response, but the bookName is internal to selectBooks
// But bookId may be empty here if the bookName is not existant....
auto bookName = mp_nameMapper->getNameForId(bookId);
std::string bookName;
switch(e.kind()) {
case ErrorKind::ArchiveNotFound:
bookName = mp_nameMapper->getNameForId(e.what());
break;
case ErrorKind::BookNotFound:
bookName = e.what();
break;
default:
break;
}
return HTTP404HtmlResponse(*this, request)
+ noSuchBookErrorMsg(bookName)
+ TaskbarInfo(bookName);
Expand Down Expand Up @@ -630,29 +667,16 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
}

try {
auto searchInfo = SearchInfo(request);
std::set<std::string> bookIds;
bookIds = selectBooks(request, "books.");
auto searchInfo = SearchInfo(request, bookIds);

/* Make the search */
// Try to get a search from the searchInfo, else build it
std::shared_ptr<zim::Search> search;
try {
search = searchCache.getOrPut(searchInfo,
[=](){
std::set<std::string> bookIds;
if(!searchInfo.bookNames.empty()) {
for(const auto& bookName: searchInfo.bookNames) {
try {
auto bookId = mp_nameMapper->getIdForName(bookName);
bookIds.insert(bookId);
} catch(const std::out_of_range&) {
throw std::invalid_argument("The requested book desn't exist.");
}
}
} else {
for (auto& bookId: mp_library->filter(kiwix::Filter().local(true).valid(true))) {
bookIds.insert(bookId);
}
}
auto searcher = mp_library->getSearcherByIds(bookIds);
return make_shared<zim::Search>(searcher->search(searchInfo.getZimQuery(m_verbose.load())));
}
Expand All @@ -664,16 +688,15 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
data.set("pattern", encodeDiples(searchInfo.pattern));
auto response = ContentResponse::build(*this, RESOURCE::templates::no_search_result_html, data, "text/html; charset=utf-8");
response->set_code(MHD_HTTP_NOT_FOUND);
if(searchInfo.bookNames.size() == 1) {
auto bookName = *searchInfo.bookNames.begin();
auto bookId = mp_nameMapper->getIdForName(bookName);
if(bookIds.size() == 1) {
auto bookId = *bookIds.begin();
auto bookName = mp_nameMapper->getNameForId(bookId);
return withTaskbarInfo(bookName, mp_library->getArchiveById(bookId).get(), std::move(response));
} else {
return std::move(response);
}
}


auto start = 0;
try {
start = request.get_argument<unsigned int>("start");
Expand All @@ -700,13 +723,31 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
renderer.setSearchProtocolPrefix(m_root + "/search?");
renderer.setPageLength(pageLength);
auto response = ContentResponse::build(*this, renderer.getHtml(), "text/html; charset=utf-8");
if(searchInfo.bookNames.size() == 1) {
auto bookName = *searchInfo.bookNames.begin();
auto bookId = mp_nameMapper->getIdForName(bookName);
if(bookIds.size() == 1) {
auto bookId = *bookIds.begin();
auto bookName = mp_nameMapper->getNameForId(bookId);
return withTaskbarInfo(bookName, mp_library->getArchiveById(bookId).get(), std::move(response));
} else {
return std::move(response);
}
} catch (const Error& e) {
// We need the bookName to build the response, but the bookName is internal to selectBooks
// But bookId may be empty here if the bookName is not existant....
std::string message;
switch(e.kind()) {
case ErrorKind::ArchiveNotFound:
case ErrorKind::BookNotFound:
message = "The requested book doesn't exist.";
break;
case ErrorKind::InvalidRequest:
message = "No query provided.";
break;
default:
break;
}
return HTTP400HtmlResponse(*this, request)
+ invalidUrlMsg
+ std::string(message);
} catch (const std::invalid_argument& e) {
return HTTP400HtmlResponse(*this, request)
+ invalidUrlMsg
Expand Down
10 changes: 5 additions & 5 deletions src/server/internalServer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,20 @@ class SearchInfo {
public:
SearchInfo(const std::string& pattern);
SearchInfo(const std::string& pattern, GeoQuery geoQuery);
SearchInfo(const RequestContext& request);
SearchInfo(const RequestContext& request, const std::set<std::string>& bookIds);

zim::Query getZimQuery(bool verbose) const;

friend bool operator<(const SearchInfo& l, const SearchInfo& r)
{
return std::tie(l.bookNames, l.pattern, l.geoQuery)
< std::tie(r.bookNames, r.pattern, r.geoQuery); // keep the same order
return std::tie(l.bookIds, l.pattern, l.geoQuery)
< std::tie(r.bookIds, r.pattern, r.geoQuery); // keep the same order
}

public: //data
std::string pattern;
GeoQuery geoQuery;
std::set<std::string> bookNames;
std::set<std::string> bookIds;
};


Expand Down Expand Up @@ -149,7 +149,7 @@ class InternalServer {

bool etag_not_needed(const RequestContext& r) const;
ETag get_matching_if_none_match_etag(const RequestContext& request) const;
std::set<std::string> selectBooks(const RequestContext& r, const std::string& prefix) const;
std::set<std::string> selectBooks(const RequestContext& r, const std::string& prefix, bool mandatory=false) const;

private: // data
std::string m_addr;
Expand Down

0 comments on commit d72ea4c

Please sign in to comment.