Fix bad web search error messages (#2034)

* fix bad web search error messages * add changelog * fix localization * fix issues * add JavaDoc, change parameter type of showErrorMessage * show sax error to user * print localized message * remove unused variable * reformat * fix issues
JabRef · Oct 3, 2016 · 816d30c · 816d30c
1 parent 3456255
commit 816d30c
Show file tree

Hide file tree

Showing 36 changed files with 259 additions and 372 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -62,6 +62,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - Fixed [#1993](https://github.com/JabRef/jabref/issues/1993): Various optimizations regarding search performance
 - Fixed [koppor#160](https://github.com/koppor/jabref/issues/160): Tooltips now working in the main table
 - Fixed [#2054](https://github.com/JabRef/jabref/issues/2054): Ignoring a new version now works as expected
+- Fixed [#1542](https://github.com/JabRef/jabref/issues/1542): Improved error messages when using fetcher
 - Fixed selecting an entry out of multiple duplicates
 - Fixed [#617](https://github.com/JabRef/jabref/issues/617): `Enter` in global search opens the selected entry & `Enter` in search dialog window opens the selected entry
 - Entries in the SearchResultPanel will be shown correctly (Latex to Unicode)

diff --git a/src/main/java/net/sf/jabref/gui/importer/FetcherPreviewDialog.java b/src/main/java/net/sf/jabref/gui/importer/FetcherPreviewDialog.java
@@ -302,4 +302,14 @@ public void showMessage(String message, String title, int msgType) {
     public void showMessage(String message) {
         JOptionPane.showMessageDialog(this, message);
     }
+
+    /**
+     * Displays a dialog which tells the user that an error occurred while fetching entries
+     */
+    public void showErrorMessage(String fetcherTitle, String localizedMessage) {
+        showMessage(Localization.lang("Error while fetching from %0", fetcherTitle) + "\n" +
+                        Localization.lang("Please try again later and/or check your network connection.") + "\n" +
+                        localizedMessage,
+                Localization.lang("Search %0", fetcherTitle), JOptionPane.ERROR_MESSAGE);
+    }
 }
diff --git a/src/main/java/net/sf/jabref/gui/importer/ImportInspectionDialog.java b/src/main/java/net/sf/jabref/gui/importer/ImportInspectionDialog.java
@@ -1472,6 +1472,16 @@ public void showMessage(String message) {
         JOptionPane.showMessageDialog(this, message);
     }
 
+    /**
+     * Displays a dialog which tells the user that an error occurred while fetching entries
+     */
+    public void showErrorMessage(String fetcherTitle, String localizedException) {
+        showMessage(Localization.lang("Error while fetching from %0", fetcherTitle) + "\n" +
+                        Localization.lang("Please try again later and/or check your network connection.") + "\n" +
+                        localizedException,
+                Localization.lang("Search %0", fetcherTitle), JOptionPane.ERROR_MESSAGE);
+    }
+
     public JabRefFrame getFrame() {
         return frame;
     }

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/ACMPortalFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/ACMPortalFetcher.java
@@ -5,7 +5,6 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
-import java.net.ConnectException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
@@ -138,8 +137,7 @@ public boolean processQueryGetPreview(String query, FetcherPreviewDialog preview
             }
 
             if (hits == 0) {
-                status.showMessage(Localization.lang("No entries found for the search string '%0'",
-                        terms),
+                status.showMessage(Localization.lang("No entries found for the search string '%0'", terms),
                         Localization.lang("Search %0", getTitle()), JOptionPane.INFORMATION_MESSAGE);
                 return false;
             } else if (hits > 20) {
@@ -157,19 +155,11 @@ public boolean processQueryGetPreview(String query, FetcherPreviewDialog preview
 
             return true;
 
-        } catch (MalformedURLException e) {
-            LOGGER.warn("Problem with ACM fetcher URL", e);
-        } catch (ConnectException e) {
-            status.showMessage(Localization.lang("Could not connect to %0", getTitle()),
-                    Localization.lang("Search %0", getTitle()), JOptionPane.ERROR_MESSAGE);
-            LOGGER.warn("Problem with ACM connection", e);
         } catch (IOException e) {
-            status.showMessage(e.getMessage(),
-                    Localization.lang("Search %0", getTitle()), JOptionPane.ERROR_MESSAGE);
-            LOGGER.warn("Problem with ACM Portal", e);
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            preview.showErrorMessage(this.getTitle(), e.getLocalizedMessage());
+            return false;
         }
-        return false;
-
     }
 
     @Override

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/CiteSeerXFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/CiteSeerXFetcher.java
@@ -11,6 +11,7 @@
 import javax.swing.JPanel;
 
 import net.sf.jabref.Globals;
+import net.sf.jabref.gui.importer.ImportInspectionDialog;
 import net.sf.jabref.logic.formatter.bibtexfields.NormalizeNamesFormatter;
 import net.sf.jabref.logic.help.HelpFile;
 import net.sf.jabref.logic.importer.ImportInspector;
@@ -63,7 +64,8 @@ public boolean processQuery(String query, ImportInspector inspector, OutputPrint
 
             return true;
         } catch (IOException e) {
-            LOGGER.warn("Could not download", e);
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            ((ImportInspectionDialog)inspector).showErrorMessage(this.getTitle(), e.getLocalizedMessage());
             return false;
         }
     }

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/DBLPFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/DBLPFetcher.java
@@ -9,6 +9,7 @@
 import javax.swing.JPanel;
 
 import net.sf.jabref.Globals;
+import net.sf.jabref.gui.importer.ImportInspectionDialog;
 import net.sf.jabref.logic.help.HelpFile;
 import net.sf.jabref.logic.importer.ImportInspector;
 import net.sf.jabref.logic.importer.OutputPrinter;
@@ -40,12 +41,9 @@ public void stopFetching() {
     }
 
     @Override
-    public boolean processQuery(String newQuery, ImportInspector inspector,
-            OutputPrinter status) {
+    public boolean processQuery(String newQuery, ImportInspector inspector, OutputPrinter status) {
 
         final HashMap<String, Boolean> bibentryKnown = new HashMap<>();
-
-        boolean res = false;
         this.query = newQuery;
 
         shouldContinue = true;
@@ -122,20 +120,16 @@ public boolean processQuery(String newQuery, ImportInspector inspector,
                 inspector.setProgress(count, bibtexUrlList.size());
                 count++;
             }
-
-
-            // everything went smooth
-            res = true;
+            return true;
 
         } catch (IOException e) {
-            LOGGER.warn("Communcation problems", e);
-            status.showMessage(e.getMessage());
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            ((ImportInspectionDialog)inspector).showErrorMessage(this.getTitle(), e.getLocalizedMessage());
         } finally {
             // Restore the threshold
             DuplicateCheck.duplicateThreshold = saveThreshold;
         }
-
-        return res;
+        return false;
     }
 
     private String makeSearchURL() {

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/DOAJFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/DOAJFetcher.java
@@ -4,6 +4,7 @@
 import javax.swing.JPanel;
 
 import net.sf.jabref.Globals;
+import net.sf.jabref.gui.importer.ImportInspectionDialog;
 import net.sf.jabref.logic.help.HelpFile;
 import net.sf.jabref.logic.importer.ImportInspector;
 import net.sf.jabref.logic.importer.OutputPrinter;
@@ -43,31 +44,25 @@ public boolean processQuery(String query, ImportInspector inspector, OutputPrint
             HttpResponse<JsonNode> jsonResponse;
             jsonResponse = Unirest.get(SEARCH_URL + query + "?pageSize=1").header("accept", "application/json").asJson();
             JSONObject jo = jsonResponse.getBody().getObject();
-            int hits = jo.getInt("total");
-            int numberToFetch = 0;
-            if (hits > 0) {
-                if (hits > MAX_PER_PAGE) {
-                    while (true) {
-                        String strCount = JOptionPane
-                                .showInputDialog(
-                                        Localization.lang("References found") + ": " + hits + "  "
-                                                + Localization.lang("Number of references to fetch?"),
-                                        Integer.toString(hits));
+            int numberToFetch = jo.getInt("total");
+            if (numberToFetch > 0) {
+                if (numberToFetch > MAX_PER_PAGE) {
+                    boolean numberEntered = false;
+                    do {
+                        String strCount = JOptionPane.showInputDialog(Localization.lang("%0 references found. Number of references to fetch?", String.valueOf(numberToFetch)));
 
                         if (strCount == null) {
-                            status.setStatus(Localization.lang("%0 import canceled", "DOAJ"));
+                            status.setStatus(Localization.lang("%0 import canceled", getTitle()));
                             return false;
                         }
 
                         try {
                             numberToFetch = Integer.parseInt(strCount.trim());
-                            break;
+                            numberEntered = true;
                         } catch (NumberFormatException ex) {
                             status.showMessage(Localization.lang("Please enter a valid number"));
                         }
-                    }
-                } else {
-                    numberToFetch = hits;
+                    } while (!numberEntered);
                 }
 
                 int fetched = 0; // Keep track of number of items fetched for the progress bar
@@ -95,15 +90,14 @@ public boolean processQuery(String query, ImportInspector inspector, OutputPrint
                 return true;
             } else {
                 status.showMessage(Localization.lang("No entries found for the search string '%0'", query),
-                        Localization.lang("Search %0", "DOAJ"), JOptionPane.INFORMATION_MESSAGE);
+                        Localization.lang("Search %0", getTitle()), JOptionPane.INFORMATION_MESSAGE);
                 return false;
             }
         } catch (UnirestException e) {
-            LOGGER.warn("Problem searching DOAJ", e);
-            status.setStatus(Localization.lang("%0 import canceled", "DOAJ"));
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            ((ImportInspectionDialog)inspector).showErrorMessage(this.getTitle(), e.getLocalizedMessage());
             return false;
         }
-
     }
 
     @Override

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/GeneralFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/GeneralFetcher.java
@@ -213,11 +213,11 @@ public void actionPerformed(ActionEvent e) {
                     activeFetcher.getTitle(), false);
             dialog.addCallBack(activeFetcher);
             dialog.setLocationRelativeTo(frame);
-            dialog.setVisible(true);
 
             JabRefExecutorService.INSTANCE.execute(() -> {
                 if (activeFetcher.processQuery(tf.getText().trim(), dialog, dialog)) {
                     dialog.entryListComplete();
+                    dialog.setVisible(true);
                 } else {
                     dialog.dispose();
                 }

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/GoogleScholarFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/GoogleScholarFetcher.java
@@ -13,6 +13,7 @@
 import java.util.regex.Pattern;
 
 import javax.swing.JLabel;
+import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 
 import net.sf.jabref.Globals;
@@ -75,16 +76,20 @@ public boolean processQueryGetPreview(String query, FetcherPreviewDialog preview
         stopFetching = false;
         try {
             Map<String, JLabel> citations = getCitations(query);
-            for (Map.Entry<String, JLabel> linkEntry : citations.entrySet()) {
-                preview.addEntry(linkEntry.getKey(), linkEntry.getValue());
+            if (!citations.isEmpty()) {
+                for (Map.Entry<String, JLabel> linkEntry : citations.entrySet()) {
+                    preview.addEntry(linkEntry.getKey(), linkEntry.getValue());
+                }
+                return true;
             }
 
-            return true;
+            status.showMessage(Localization.lang("No entries found for the search string '%0'", query),
+                    Localization.lang("Search %0", getTitle()), JOptionPane.INFORMATION_MESSAGE);
         } catch (IOException e) {
-            LOGGER.warn("Error fetching from Google Scholar", e);
-            status.showMessage(Localization.lang("Error while fetching from %0", "Google Scholar"));
-            return false;
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            preview.showErrorMessage(this.getTitle(), e.getLocalizedMessage());
         }
+        return false;
     }
 
     @Override

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/IEEEXploreFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/IEEEXploreFetcher.java
@@ -2,11 +2,8 @@
 
 import java.awt.BorderLayout;
 import java.io.IOException;
-import java.net.ConnectException;
 import java.net.CookieHandler;
 import java.net.CookieManager;
-import java.net.MalformedURLException;
-import java.net.UnknownHostException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -22,6 +19,7 @@
 import javax.swing.JPanel;
 
 import net.sf.jabref.Globals;
+import net.sf.jabref.gui.importer.ImportInspectionDialog;
 import net.sf.jabref.logic.formatter.bibtexfields.HtmlToLatexFormatter;
 import net.sf.jabref.logic.formatter.bibtexfields.UnitsToLatexFormatter;
 import net.sf.jabref.logic.formatter.casechanger.ProtectTermsFormatter;
@@ -150,11 +148,6 @@ public boolean processQuery(String query, ImportInspector dialog, OutputPrinter
             //parse the page into Bibtex entries
             Collection<BibEntry> parsedBibtexCollection = BibtexParser.fromString(bibtexPage,
                     Globals.prefs.getImportFormatPreferences());
-            if (parsedBibtexCollection == null) {
-                status.showMessage(Localization.lang("Error while fetching from %0", getTitle()),
-                        DIALOG_TITLE, JOptionPane.INFORMATION_MESSAGE);
-                return false;
-            }
             int nEntries = parsedBibtexCollection.size();
             Iterator<BibEntry> parsedBibtexCollectionIterator = parsedBibtexCollection.iterator();
             while (parsedBibtexCollectionIterator.hasNext() && shouldContinue) {
@@ -165,14 +158,9 @@ public boolean processQuery(String query, ImportInspector dialog, OutputPrinter
 
             return true;
 
-        } catch (MalformedURLException e) {
-            LOGGER.warn("Bad URL", e);
-        } catch (ConnectException | UnknownHostException e) {
-            status.showMessage(Localization.lang("Could not connect to %0", getTitle()), DIALOG_TITLE,
-                    JOptionPane.ERROR_MESSAGE);
         } catch (IOException | JSONException e) {
-            status.showMessage(e.getMessage(), DIALOG_TITLE, JOptionPane.ERROR_MESSAGE);
-            LOGGER.warn("Search IEEEXplore: " + e.getMessage(), e);
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            ((ImportInspectionDialog)dialog).showErrorMessage(this.getTitle(), e.getLocalizedMessage());
         }
 
         return false;

diff --git a/src/main/java/net/sf/jabref/gui/importer/fetcher/INSPIREFetcher.java b/src/main/java/net/sf/jabref/gui/importer/fetcher/INSPIREFetcher.java
@@ -7,13 +7,12 @@
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLEncoder;
-import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 
-import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 
 import net.sf.jabref.Globals;
+import net.sf.jabref.gui.importer.ImportInspectionDialog;
 import net.sf.jabref.logic.help.HelpFile;
 import net.sf.jabref.logic.importer.ImportInspector;
 import net.sf.jabref.logic.importer.OutputPrinter;
@@ -100,25 +99,18 @@ private String constructUrl(String key) {
      * @param key The OAI2 key to fetch from ArXiv.
      * @return The imported BibEntry or null if none.
      */
-    private BibDatabase importInspireEntries(String key, OutputPrinter frame) {
+    private BibDatabase importInspireEntries(String key, OutputPrinter frame) throws IOException {
         String url = constructUrl(key);
-        try {
-            HttpURLConnection conn = (HttpURLConnection) new URL(url).openConnection();
-            conn.setRequestProperty("User-Agent", "JabRef");
-            InputStream inputStream = conn.getInputStream();
-
-            try (INSPIREBibtexFilterReader reader = new INSPIREBibtexFilterReader(
-                    new InputStreamReader(inputStream, Charset.forName("UTF-8")))) {
+        HttpURLConnection conn = (HttpURLConnection) new URL(url).openConnection();
+        conn.setRequestProperty("User-Agent", "JabRef");
+        InputStream inputStream = conn.getInputStream();
 
-                ParserResult pr = BibtexParser.parse(reader, Globals.prefs.getImportFormatPreferences());
+        try (INSPIREBibtexFilterReader reader = new INSPIREBibtexFilterReader(
+                new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
 
-                return pr.getDatabase();
-            }
-        } catch (RuntimeException | IOException e) {
-            frame.showMessage(Localization.lang("An exception occurred while accessing '%0'", url) + "\n\n" + e,
-                    getTitle(), JOptionPane.ERROR_MESSAGE);
+            ParserResult pr = BibtexParser.parse(reader, Globals.prefs.getImportFormatPreferences());
+            return pr.getDatabase();
         }
-        return null;
     }
 
 
@@ -150,24 +142,20 @@ public void stopFetching() {
      * @see java.lang.Runnable
      */
     @Override
-    public boolean processQuery(String query, ImportInspector dialog, OutputPrinter frame) {
+    public boolean processQuery(String query, ImportInspector dialog, OutputPrinter status) {
         try {
-            frame.setStatus(Localization.lang("Fetching entries from Inspire"));
+            status.setStatus(Localization.lang("Fetching entries from Inspire"));
             /* query the archive and load the results into the BibEntry */
-            BibDatabase bd = importInspireEntries(query, frame);
+            BibDatabase bd = importInspireEntries(query, status);
 
-            frame.setStatus(Localization.lang("Adding fetched entries"));
+            status.setStatus(Localization.lang("Adding fetched entries"));
             /* add the entry to the inspection dialog */
-            if (bd == null) {
-                LOGGER.warn("Error while fetching from Inspire");
-            } else {
-                bd.getEntries().forEach(dialog::addEntry);
-            }
-            /* inform the inspection dialog, that we're done */
+            bd.getEntries().forEach(dialog::addEntry);
+            return true;
         } catch (Exception e) {
-            frame.showMessage(Localization.lang("Error while fetching from %0", "Inspire") + ": " + e.getMessage());
-            LOGGER.warn("Error while fetching from Inspire", e);
+            LOGGER.error("Error while fetching from " + getTitle(), e);
+            ((ImportInspectionDialog)dialog).showErrorMessage(this.getTitle(), e.getLocalizedMessage());
         }
-        return true;
+        return false;
     }
 }