Uploaded image for project: 'Solr Search Provider'
  1. Solr Search Provider
  2. MGNLEESOLR-44

Improve error handling - always log root exceptions and not just their messages

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Fixed
    • Neutral
    • 2.1
    • 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

        Attachments

          Activity

            People

              mdivilek Milan Divilek
              edgar Edgar Vonk
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Checklists

                  Task DoD