[MGNLEESOLR-44] Improve error handling - always log root exceptions and not just their messages Created: 11/Feb/15 Updated: 10/Mar/15 Resolved: 09/Mar/15 |
|
| Status: | Closed |
| Project: | Solr Search Provider |
| Component/s: | None |
| Affects Version/s: | 2.0 |
| Fix Version/s: | 2.1 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Edgar Vonk | Assignee: | Milan Divilek |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | quickwin | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Template: |
|
| Acceptance criteria: |
Empty
|
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
| 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. |