diff --git a/pom.xml b/pom.xml index 0af43488f1..dde789075d 100644 --- a/pom.xml +++ b/pom.xml @@ -46,6 +46,14 @@ + + org.apache.maven.plugins + maven-compiler-plugin + + 1.6 + 1.6 + + org.codehaus.mojo findbugs-maven-plugin diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 77d178800c..850edaf531 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.text.ParseException; @@ -55,6 +56,8 @@ import com.fasterxml.jackson.databind.introspect.VisibilityChecker.Std; import com.infradna.tool.bridge_method_injector.WithBridgeMethods; import java.nio.charset.Charset; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Root of the GitHub API. @@ -68,6 +71,8 @@ * @author Kohsuke Kawaguchi */ public class GitHub { + protected final transient Logger logger = Logger.getLogger(getClass().getName()); + /*package*/ final String login; /** @@ -458,6 +463,8 @@ public boolean isCredentialValid() throws IOException { retrieve().to("/user", GHUser.class); return true; } catch (IOException e) { + if (logger.isLoggable(Level.FINE)) + logger.log(Level.FINE, "Exception validating credentials on " + this.apiUrl + " with login '" + this.login + "' " + e, e); return false; } } diff --git a/src/main/java/org/kohsuke/github/HttpException.java b/src/main/java/org/kohsuke/github/HttpException.java new file mode 100644 index 0000000000..718642299d --- /dev/null +++ b/src/main/java/org/kohsuke/github/HttpException.java @@ -0,0 +1,118 @@ +package org.kohsuke.github; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URL; + +import javax.annotation.CheckForNull; + +/** + * {@link IOException} for http exceptions because {@link HttpURLConnection} throws un-discerned + * {@link IOException} and it can help to know the http response code to decide how to handle an + * http exceptions. + * + * @author Cyrille Le Clerc + */ +public class HttpException extends IOException { + static final long serialVersionUID = 1L; + + private final int responseCode; + private final String responseMessage; + private final String url; + + /** + * @param message The detail message (which is saved for later retrieval + * by the {@link #getMessage()} method) + * @param responseCode Http response code. {@code -1} if no code can be discerned. + * @param responseMessage Http response message + * @param url The url that was invoked + * @see HttpURLConnection#getResponseCode() + * @see HttpURLConnection#getResponseMessage() + */ + public HttpException(String message, int responseCode, String responseMessage, String url) { + super(message); + this.responseCode = responseCode; + this.responseMessage = responseMessage; + this.url = url; + } + + /** + * @param message The detail message (which is saved for later retrieval + * by the {@link #getMessage()} method) + * @param responseCode Http response code. {@code -1} if no code can be discerned. + * @param responseMessage Http response message + * @param url The url that was invoked + * @param cause The cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is permitted, + * and indicates that the cause is nonexistent or unknown.) + * @see HttpURLConnection#getResponseCode() + * @see HttpURLConnection#getResponseMessage() + */ + public HttpException(String message, int responseCode, String responseMessage, String url, Throwable cause) { + super(message, cause); + this.responseCode = responseCode; + this.responseMessage = responseMessage; + this.url = url; + } + + /** + * @param message The detail message (which is saved for later retrieval + * by the {@link #getMessage()} method) + * @param responseCode Http response code. {@code -1} if no code can be discerned. + * @param responseMessage Http response message + * @param url The url that was invoked + * @param cause The cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is permitted, + * and indicates that the cause is nonexistent or unknown.) + * @see HttpURLConnection#getResponseCode() + * @see HttpURLConnection#getResponseMessage() + */ + public HttpException(int responseCode, String responseMessage, String url, Throwable cause) { + super("Server returned HTTP response code: " + responseCode + ", message: '" + responseMessage + "'" + + " for URL: " + url, cause); + this.responseCode = responseCode; + this.responseMessage = responseMessage; + this.url = url; + } + + /** + * @param responseCode Http response code. {@code -1} if no code can be discerned. + * @param responseMessage Http response message + * @param url The url that was invoked + * @param cause The cause (which is saved for later retrieval by the + * {@link #getCause()} method). (A null value is permitted, + * and indicates that the cause is nonexistent or unknown.) + * @see HttpURLConnection#getResponseCode() + * @see HttpURLConnection#getResponseMessage() + */ + public HttpException(int responseCode, String responseMessage, @CheckForNull URL url, Throwable cause) { + this(responseCode, responseMessage, url == null ? null : url.toString(), cause); + } + + /** + * Http response code of the request that cause the exception + * + * @return {@code -1} if no code can be discerned. + */ + public int getResponseCode() { + return responseCode; + } + + /** + * Http response message of the request that cause the exception + * + * @return {@code null} if no response message can be discerned. + */ + public String getResponseMessage() { + return responseMessage; + } + + /** + * The http URL that caused the exception + * + * @return url + */ + public String getUrl() { + return url; + } +} diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java index af4819c3a1..6ac2a084c2 100644 --- a/src/main/java/org/kohsuke/github/Requester.java +++ b/src/main/java/org/kohsuke/github/Requester.java @@ -48,6 +48,8 @@ import java.util.Locale; import java.util.Map; import java.util.NoSuchElementException; +import java.util.logging.Level; +import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.zip.GZIPInputStream; @@ -64,7 +66,9 @@ */ class Requester { private static final List METHODS_WITHOUT_BODY = asList("GET", "DELETE"); - + + protected final transient Logger logger = Logger.getLogger(getClass().getName()); + private final GitHub root; private final List args = new ArrayList(); private final Map headers = new LinkedHashMap(); @@ -473,21 +477,33 @@ private void setupConnection(URL url) throws IOException { } private T parse(Class type, T instance) throws IOException { - if (uc.getResponseCode()==304) - return null; // special case handling for 304 unmodified, as the content will be "" InputStreamReader r = null; + int responseCode = -1; + String responseMessage = null; try { + responseCode = uc.getResponseCode(); + responseMessage = uc.getResponseMessage(); + if (responseCode == 304) { + return null; // special case handling for 304 unmodified, as the content will be "" + } + r = new InputStreamReader(wrapStream(uc.getInputStream()), "UTF-8"); String data = IOUtils.toString(r); if (type!=null) try { return MAPPER.readValue(data,type); } catch (JsonMappingException e) { - throw (IOException)new IOException("Failed to deserialize "+data).initCause(e); + throw (IOException)new IOException("Failed to deserialize " +data).initCause(e); } if (instance!=null) return MAPPER.readerForUpdating(instance).readValue(data); return null; + } catch (FileNotFoundException e) { + // java.net.URLConnection handles 404 exception has FileNotFoundException, don't wrap exception in HttpException + // to preserve backward compatibility + throw e; + } catch (IOException e) { + throw new HttpException(responseCode, responseMessage, uc.getURL(), e); } finally { IOUtils.closeQuietly(r); } @@ -508,7 +524,18 @@ private InputStream wrapStream(InputStream in) throws IOException { * Handle API error by either throwing it or by returning normally to retry. */ /*package*/ void handleApiError(IOException e) throws IOException { - if (uc.getResponseCode() == 401) // Unauthorized == bad creds + int responseCode; + try { + responseCode = uc.getResponseCode(); + } catch (IOException e2) { + // likely to be a network exception (e.g. SSLHandshakeException), + // uc.getResponseCode() and any other getter on the response will cause an exception + if (logger.isLoggable(Level.FINE)) + logger.log(Level.FINE, "Silently ignore exception retrieving response code for '" + uc.getURL() + "'" + + " handling exception " + e, e); + throw e; + } + if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED) // 401 / Unauthorized == bad creds throw e; if ("0".equals(uc.getHeaderField("X-RateLimit-Remaining"))) {