Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issuexxxx improve http error handling #1033

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions controller/WebController.php
Original file line number Diff line number Diff line change
Expand Up @@ -337,23 +337,47 @@ public function invokeGlobalSearch($request)
$vocabObjects = array();
if ($vocids) {
foreach($vocids as $vocid) {
$vocabObjects[] = $this->model->getVocabulary($vocid);
try {
$vocabObjects[] = $this->model->getVocabulary($vocid);
} catch (Exception $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty catch block is bad practice. What happens with the $vocabObjects variable that is used below?
At a minimum, it could be set to null here, or array() if that makes more sense.

// skip vocabularies not found in configuration
// please note that this may result in global search
// NB: should not happen in normal UI interaction
}
}
}
$parameters->setVocabularies($vocabObjects);

$nondefaultEndpointVocs = array();

if (sizeOf($vocabObjects) != 1) {
// global search, either from all (sizeOf($vocabObjects) == 0) or from selected ones
if (sizeOf($vocabObjects) == 0) {
$vocabObjects = $this->model->getVocabularies();
}
$defaultEndpoint = $this->model->getConfig()->getDefaultEndpoint();
foreach($vocabObjects as $voc) {
if ($voc->getEndpoint() !== $defaultEndpoint) {
$nondefaultEndpointVocs[] = $voc;
}
}
}

$counts = null;
$searchResults = null;
$errored = false;

try {
$countAndResults = $this->model->searchConceptsAndInfo($parameters);
$counts = $countAndResults['count'];
$searchResults = $countAndResults['results'];
} catch (Exception $e) {
header("HTTP/1.0 404 Not Found");
$errored = true;
header("HTTP/1.0 500 Internal Server Error");
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
$this->invokeGenericErrorPage($request, $e->getMessage());
return;
}
$counts = $countAndResults['count'];
$searchResults = $countAndResults['results'];
$vocabList = $this->model->getVocabularyList();
$sortedVocabs = $this->model->getVocabularyList(false, true);
$langList = $this->model->getLanguages($lang);
Expand All @@ -365,6 +389,8 @@ public function invokeGlobalSearch($request)
'search_results' => $searchResults,
'rest' => $parameters->getOffset()>0,
'global_search' => true,
'search_failed' => $errored,
'skipped_vocabs' => $nondefaultEndpointVocs,
'term' => $request->getQueryParamRaw('q'),
'lang_list' => $langList,
'vocabs' => str_replace(' ', '+', $vocabs),
Expand All @@ -383,17 +409,21 @@ public function invokeVocabularySearch($request)
$template = $this->twig->loadTemplate('vocab-search-listing.twig');
$this->setLanguageProperties($request->getLang());
$vocab = $request->getVocab();
$searchResults = null;
try {
$vocabTypes = $this->model->getTypes($request->getVocabid(), $request->getLang());
} catch (Exception $e) {
header("HTTP/1.0 404 Not Found");
header("HTTP/1.0 500 Internal Server Error");
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}

echo $template->render(
array(
'languages' => $this->languages,
'vocab' => $vocab,
'request' => $request,
'search_results' => $searchResults
));

return;
Expand All @@ -417,6 +447,7 @@ public function invokeVocabularySearch($request)
'languages' => $this->languages,
'vocab' => $vocab,
'term' => $request->getQueryParam('q'),
'search_results' => $searchResults
));
return;
}
Expand Down Expand Up @@ -548,6 +579,7 @@ public function invokeGenericErrorPage($request, $message = null)
array(
'languages' => $this->languages,
'request' => $request,
'vocab' => $request->getVocab(),
'message' => $message,
'requested_page' => filter_input(INPUT_SERVER, 'REQUEST_URI', FILTER_SANITIZE_STRING),
));
Expand Down
25 changes: 18 additions & 7 deletions model/DataObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,15 @@ public function __construct($model, $resource)
protected function getExternalLabel($exvoc, $exuri, $lang)
{
if ($exvoc) {
$exsparql = $exvoc->getSparql();
$results = $exsparql->queryLabel($exuri, $lang);

return isset($results[$lang]) ? $results[$lang] : null;
try {
$exsparql = $exvoc->getSparql();
$results = $exsparql->queryLabel($exuri, $lang);
return isset($results[$lang]) ? $results[$lang] : null;
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
}
}
return null;
}
Expand All @@ -63,9 +68,15 @@ protected function getExternalLabel($exvoc, $exuri, $lang)
protected function getExternalNotation($exvoc, $exuri)
{
if ($exvoc) {
$exsparql = $exvoc->getSparql();
$results = $exsparql->queryNotation($exuri);
return isset($results) ? $results : null;
try {
$exsparql = $exvoc->getSparql();
$results = $exsparql->queryNotation($exuri);
return isset($results) ? $results : null;
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
}
}
return null;
}
Expand Down
29 changes: 22 additions & 7 deletions model/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,18 @@ private function disambiguateVocabulary($vocabs, $uri, $preferredVocabId = null)
if($preferredVocabId != null) {
foreach ($vocabs as $vocab) {
if($vocab->getId() == $preferredVocabId) {
// double check that a label exists in the preferred vocabulary
if ($vocab->getConceptLabel($uri, null) !== null) {
return $vocab;
} else {
// not found in preferred vocabulary, fall back to next method
try {
// double check that a label exists in the preferred vocabulary
if ($vocab->getConceptLabel($uri, null) !== null) {
return $vocab;
} else {
// not found in preferred vocabulary, fall back to next method
break;
}
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
break;
}
}
Expand All @@ -483,8 +490,16 @@ private function disambiguateVocabulary($vocabs, $uri, $preferredVocabId = null)

// no preferred vocabulary, or it was not found, search in which vocabulary the concept has a label
foreach ($vocabs as $vocab) {
if ($vocab->getConceptLabel($uri, null) !== null)
return $vocab;
try {
if ($vocab->getConceptLabel($uri, null) !== null) {
return $vocab;
}
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
break;
}
}

// if the URI couldn't be found, fall back to the first vocabulary
Expand Down
41 changes: 31 additions & 10 deletions model/Vocabulary.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,19 @@ public function getInfo($lang = null)
}
}

// also include ConceptScheme metadata from SPARQL endpoint
$defaultcs = $this->getDefaultConceptScheme();

// query everything the endpoint knows about the ConceptScheme
$sparql = $this->getSparql();
$result = $sparql->queryConceptScheme($defaultcs);
try {
// also include ConceptScheme metadata from SPARQL endpoint
$defaultcs = $this->getDefaultConceptScheme();

// query everything the endpoint knows about the ConceptScheme
$sparql = $this->getSparql();
$result = $sparql->queryConceptScheme($defaultcs);
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
return null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ok?

}
$conceptscheme = $result->resource($defaultcs);
$this->order = array("dc:title", "dc11:title", "skos:prefLabel", "rdfs:label", "dc:subject", "dc11:subject", "dc:description", "dc11:description", "dc:publisher", "dc11:publisher", "dc:creator", "dc11:creator", "dc:contributor", "dc:language", "dc11:language", "owl:versionInfo", "dc:source", "dc11:source");

Expand Down Expand Up @@ -227,8 +234,15 @@ public function getConceptSchemes($lang = '')
if ($lang === '') {
$lang = $this->getEnvLang();
}

return $this->getSparql()->queryConceptSchemes($lang);
$conceptSchemes = null;
try {
$conceptSchemes = $this->getSparql()->queryConceptSchemes($lang);
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
}
return $conceptSchemes;
}

/**
Expand Down Expand Up @@ -418,8 +432,15 @@ public function getConceptRelateds($uri, $lang)
public function getConceptInfo($uri, $clang)
{
$sparql = $this->getSparql();

return $sparql->queryConceptInfo($uri, $this->config->getArrayClassURI(), array($this), $clang);
$conceptInfo = null;
try {
$conceptInfo = $sparql->queryConceptInfo($uri, $this->config->getArrayClassURI(), array($this), $clang);
} catch (EasyRdf\Http\Exception | EasyRdf\Exception | Throwable $e) {
if ($this->model->getConfig()->getLogCaughtExceptions()) {
error_log('Caught exception: ' . $e->getMessage());
}
}
return $conceptInfo;
}

/**
Expand Down
27 changes: 27 additions & 0 deletions resource/css/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,14 @@ ul.dropdown-menu > li:last-child > input {
margin: 15px;
}

.search-result-listing > .alert {
margin: 10px 15px;
}

.search-result-listing > .alert > h4 {
margin: 10px 0px;
}

.search-result-listing .search-count {
text-align: left;
margin-left: 15px;
Expand Down Expand Up @@ -935,6 +943,21 @@ div#sidebar-grey > div > div > form.search-options {
top: 10px;
}

.search-result-listing .alert .btn-default {
position: unset;
vertical-align: unset;
margin-left: 15px;
font-size: 18px;
}

.alert h4, .alert h3 {
display: inline;
}

.search-result-listing .alert-warning h4 {
display: block;
}

.search-options .mCSB_container > li > a > .radio {
margin: 0;
padding-left: 25px;
Expand Down Expand Up @@ -1066,6 +1089,10 @@ li.sub-group {
right: 0;
}

.frontpage-alert > h3, .page-alert > h3 {
margin: 10px 0px;
}

.alert h2 {
margin: 5px 0 5px 0;
}
Expand Down
29 changes: 23 additions & 6 deletions resource/js/docready.js
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ $(function() { // DOCUMENT READY
$('.search-result-listing').append($ready);
}
else {
$trigger.waypoint(function() { waypointCallback(); }, options);
$trigger.waypoint(function() { waypointCallback(this); }, options);
}
}

Expand Down Expand Up @@ -881,9 +881,13 @@ $(function() { // DOCUMENT READY
changeOffset += 200;
}

function waypointCallback() {
function waypointCallback(waypoint) {
if ($('.search-result-listing > p .spinner,.search-result-listing .alert-danger').length > 0) {
return false;
}
var number_of_hits = $(".search-result").length;
if (number_of_hits < parseInt($('.search-count p').text().substr(0, $('.search-count p').text().indexOf(' ')), 10)) { $('.search-result-listing').append($loading);
if (number_of_hits < parseInt($('.search-count p').text().substr(0, $('.search-count p').text().indexOf(' ')), 10)) {
$('.search-result-listing').append($loading);
var typeLimit = $('#type-limit').val();
var schemeLimit = $('#scheme-limit').val();
var groupLimit = $('#group-limit').val();
Expand All @@ -903,7 +907,20 @@ $(function() { // DOCUMENT READY
$('.search-result-listing').append($ready);
return false;
}
$('.search-result:nth-last-of-type(4)').waypoint(function() { waypointCallback(); }, options );
waypoint.destroy();
$('.search-result:nth-last-of-type(4)').waypoint(function() { waypointCallback(this); }, options );
},
error: function(jqXHR, textStatus, errorThrown) {
$loading.detach();
var $failedSearch = $('<div class="alert alert-danger"><h4>Error: Loading for more items failed!</h4></div>');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message should be translated

var $retryButton = $('<button class="btn btn-default" type="button">Retry</button>');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Retry" should be translated as well

$retryButton.on('click', function () {
$failedSearch.remove();
waypointCallback(waypoint);
return false;
});
$failedSearch.append($retryButton)
$('.search-result-listing').append($failedSearch);
}
});
}
Expand Down Expand Up @@ -1042,9 +1059,9 @@ $(function() { // DOCUMENT READY
}

/* makes an AJAX query for the alphabetical index contents when landing on
* the vocabulary home page.
* the vocabulary home page or on the vocabulary concept error page.
*/
if ($('#alpha').hasClass('active') && $('#vocab-info').length === 1 && $('.alphabetical-search-results').length === 0) {
if ($('#alpha').hasClass('active') && $('#vocab-info,.page-alert').length == 1 && $('.alphabetical-search-results').length == 0) {
// taking into account the possibility that the lang parameter has been changed by the WebController.
var urlLangCorrected = vocab + '/' + lang + '/index?limit=250&offset=0&clang=' + clang;
$('.sidebar-grey').empty().append('<div class="loading-spinner"><span class="spinner-text">'+ loading_text + '</span><span class="spinner" /></div>');
Expand Down
3 changes: 2 additions & 1 deletion resource/js/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ function loadLimitedResults(parameters) {
clearResultsAndAddSpinner();
$.ajax({
data: parameters,
success : function(data) {
complete : function(jqXHR, textStatus) {
var data = jqXHR.responseText;
var response = $('.search-result-listing', data).html();
if (window.history.pushState) { window.history.pushState({url: this.url}, '', this.url); }
$('.search-result-listing').append(response);
Expand Down
15 changes: 13 additions & 2 deletions view/error-page.twig
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
{% extends "light.twig" %}
{% block title %}: Error page{% endblock %}
{% block error %}
<div class="alert {% if request.vocabid != '' or request.page == 'feedback' or request.page == 'about' %}page-alert{% else %}frontpage-alert{% endif %}">
<h3>{% if not message %}{% trans %}404 Error: The page {{ requested_page }} cannot be found.{% endtrans %}{% else %}{{ message }}{% endif %}</h3>
{% spaceless %}
{% set pageError = request.vocabid != '' and request.page == 'page' %}
{% if pageError %}
<div id="maincontent">
<div class="content">
{% endif %}
<div class="alert {% if request.vocabid != '' or request.page == 'feedback' or request.page == 'about' %}page-alert{% else %}frontpage-alert{% endif %}">
<h3>{% if not message %}{% trans %}404 Error: The page {{ requested_page }} cannot be found.{% endtrans %}{% else %}{{ message }}{% endif %}</h3>
</div>
{% if pageError %}
</div>
</div>
{% endif %}
{% endspaceless %}
{% endblock %}
2 changes: 1 addition & 1 deletion view/headerbar.twig
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<div class="header-float">
{% if request.page != 'About' and request.page != 'Feedback' %}

{% if request.page != '' %}<div class="search-vocab-text"><p>{% trans "Content language" %}</p></div>{% endif %}
{% if request.page != '' and not global_search %}<div class="search-vocab-text"><p>{% trans "Content language" %}</p></div>{% endif %}
{% if sorted_vocabs is defined %}
<label class="sr-only" for="multiselect">{% trans "Search from vocabulary" %}</label>
<select class="multiselect" id="multiselect" multiple="multiple">
Expand Down
Loading