Details
-
Improvement
-
Resolution: Fixed
-
Neutral
-
2.0
Description
Hi there,
Great module! However I think the error handling can be improved in places. It took me a long time to find out (with debugging) why my Solr server configuration was giving problems for the module because the in some occasions the root exception (from SolrJ) was lost. Only the non-informative exception message was lost.
E.g. in SolrIndexerImpl:
} catch (IOException e) { log.error("Can't index {} - {} ", things , e.getMessage()); return false; } catch (SolrServerException e) { log.error("Can't index {} - {} ", things , e.getMessage()); return false; } catch (SolrException e) { log.error("Can't index {} - {} ", things , e.getMessage()); return false; }
Should, I think, be replaced by:
} catch (IOException e) { log.error("Can't index {} - {} ", things , e); return false; } catch (SolrServerException e) { log.error("Can't index {} - {} ", things , e); return false; } catch (SolrException e) { log.error("Can't index {} - {} ", things , e); return false; }
Especially in case of errors the root exception should always be logged. Logging only the error message is evil I think.
And leads to a lot of work when error occur. Especially on production systems where you cannot debug.
Checklists
Acceptance criteria