diff --git a/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java b/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java index d509ef023..cf14079cb 100644 --- a/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java +++ b/sentry-core/src/main/java/io/sentry/core/transport/HttpTransport.java @@ -6,6 +6,7 @@ import static io.sentry.core.SentryLevel.WARNING; import com.jakewharton.nopen.annotation.Open; +import io.sentry.core.ILogger; import io.sentry.core.ISerializer; import io.sentry.core.SentryEnvelope; import io.sentry.core.SentryEvent; @@ -30,6 +31,7 @@ import java.util.Date; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.zip.GZIPOutputStream; import javax.net.ssl.HttpsURLConnection; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -37,7 +39,7 @@ /** * An implementation of the {@link ITransport} interface that sends the events to the Sentry server - * over HTTP(S) in UTF-8 encoding. + * over HTTP(S). */ @Open @ApiStatus.NonExtendable // only not final because of testing @@ -84,6 +86,8 @@ public String getCategory() { private final @NotNull ICurrentDateProvider currentDateProvider; + private final @NotNull ILogger logger; + /** * Constructs a new HTTP transport instance. Notably, the provided {@code requestUpdater} must set * the appropriate content encoding header for the {@link io.sentry.core.ISerializer} instance @@ -131,6 +135,7 @@ public HttpTransport( this.bypassSecurity = bypassSecurity; this.currentDateProvider = Objects.requireNonNull(currentDateProvider, "CurrentDateProvider is required."); + this.logger = Objects.requireNonNull(options.getLogger(), "Logger is required."); try { final URI uri = sentryUrl.toURI(); @@ -159,41 +164,20 @@ public HttpTransport( int responseCode = -1; try (final OutputStream outputStream = connection.getOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) { + final GZIPOutputStream gzip = new GZIPOutputStream(outputStream); + final Writer writer = new BufferedWriter(new OutputStreamWriter(gzip, UTF_8))) { serializer.serialize(event, writer); - // need to also close the input stream of the connection - connection.getInputStream().close(); - options.getLogger().log(DEBUG, "Event sent %s successfully.", event.getEventId()); + logger.log(DEBUG, "Event sent %s successfully.", event.getEventId()); return TransportResult.success(); } catch (IOException e) { - try { - responseCode = connection.getResponseCode(); - - if (responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - options - .getLogger() - .log( - DEBUG, - "Event '" - + event.getEventId() - + "' was rejected by the Sentry server due to a filter."); - } - logErrorInPayload(connection); - return TransportResult.error(responseCode); - } catch (IOException responseCodeException) { - // this should not stop us from continuing. We'll just use -1 as response code. - options - .getLogger() - .log(WARNING, "Failed to obtain response code while analyzing event send failure.", e); - } - - logErrorInPayload(connection); - return TransportResult.error(responseCode); + final TransportResult errorResult = getResultFromErrorCode(connection, e); + responseCode = errorResult.getResponseCode(); + return errorResult; } finally { updateRetryAfterLimits(connection, responseCode); - connection.disconnect(); + closeAndDisconnect(connection); } } @@ -272,7 +256,7 @@ public boolean isRetryAfter(final @NotNull String itemType) { connection.setRequestMethod("POST"); connection.setDoOutput(true); - connection.setRequestProperty("Content-Encoding", "UTF-8"); + connection.setRequestProperty("Content-Encoding", "gzip"); connection.setRequestProperty("Content-Type", contentType); connection.setRequestProperty("Accept", "application/json"); @@ -297,46 +281,80 @@ public boolean isRetryAfter(final @NotNull String itemType) { int responseCode = -1; try (final OutputStream outputStream = connection.getOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) { + final GZIPOutputStream gzip = new GZIPOutputStream(outputStream); + final Writer writer = new BufferedWriter(new OutputStreamWriter(gzip, UTF_8))) { serializer.serialize(envelope, writer); - // need to also close the input stream of the connection - connection.getInputStream().close(); - options.getLogger().log(DEBUG, "Envelope sent successfully."); + logger.log(DEBUG, "Envelope sent successfully."); return TransportResult.success(); } catch (IOException e) { - try { - responseCode = connection.getResponseCode(); - - // TODO: 403 part of the protocol? - if (responseCode == HttpURLConnection.HTTP_FORBIDDEN) { - options - .getLogger() - .log(DEBUG, "Envelope was rejected by the Sentry server due to a filter."); - } - logErrorInPayload(connection); - return TransportResult.error(responseCode); - } catch (IOException responseCodeException) { - // this should not stop us from continuing. We'll just use -1 as response code. - options - .getLogger() - .log(WARNING, "Failed to obtain response code while analyzing event send failure.", e); - } - - logErrorInPayload(connection); - return TransportResult.error(responseCode); + final TransportResult errorResult = getResultFromErrorCode(connection, e); + responseCode = errorResult.getResponseCode(); + return errorResult; } catch (Exception e) { - options - .getLogger() - .log(WARNING, "Failed to obtain error message while analyzing event send failure.", e); - return TransportResult.error(-1); + return getDefaultErrorResult(e); } finally { updateRetryAfterLimits(connection, responseCode); + closeAndDisconnect(connection); + } + } + + /** + * Closes the Response stream and disconnect the connection + * + * @param connection the HttpURLConnection + */ + private void closeAndDisconnect(final @NotNull HttpURLConnection connection) { + try { + connection.getInputStream().close(); + } catch (IOException e) { + logger.log(ERROR, e, "Error while closing the connection."); + } finally { connection.disconnect(); } } + /** + * Returns a TransportResult error with the given responseCode + * + * @param connection the HttpURLConnection + * @param ioException the IOException + * @return the TransportResult error + */ + private @NotNull TransportResult getResultFromErrorCode( + final @NotNull HttpURLConnection connection, final @NotNull IOException ioException) { + try { + final int responseCode = connection.getResponseCode(); + + logErrorInPayload(connection); + return TransportResult.error(responseCode); + } catch (IOException responseCodeException) { + logger.log(ERROR, responseCodeException, "Error getting responseCode"); + + // this should not stop us from continuing. + return getDefaultErrorResult(ioException); + } + } + + /** + * Logs a default error message and return the TransportResult error with -1 responseCode + * + * @param exception the Exception + * @return the TransportResult error + */ + private @NotNull TransportResult getDefaultErrorResult(final @NotNull Exception exception) { + logger.log( + WARNING, "Failed to obtain error message while analyzing event send failure.", exception); + return TransportResult.error(); + } + + /** + * Read retry after headers and update the rate limit Dictionary + * + * @param connection the HttpURLConnection + * @param responseCode the responseCode + */ private void updateRetryAfterLimits( final @NotNull HttpURLConnection connection, final int responseCode) { // seconds @@ -391,7 +409,7 @@ private void updateRetryAfterLimits( try { dataCategory = DataCategory.valueOf(StringUtils.capitalize(catItem)); } catch (IllegalArgumentException e) { - options.getLogger().log(INFO, e, "Unknown category: %s", catItem); + logger.log(INFO, e, "Unknown category: %s", catItem); } // we dont apply rate limiting for unknown categories if (DataCategory.Unknown.equals(dataCategory)) { @@ -442,34 +460,42 @@ private long parseRetryAfterOrDefault(final @Nullable String retryAfterHeader) { try { retryAfterMillis = (long) (Double.parseDouble(retryAfterHeader) * 1000L); // seconds -> milliseconds - } catch (NumberFormatException __) { + } catch (NumberFormatException ignored) { // let's use the default then } } return retryAfterMillis; } + /** + * Logs the error message from the ErrorStream + * + * @param connection the HttpURLConnection + */ private void logErrorInPayload(final @NotNull HttpURLConnection connection) { - if (options - .isDebug()) { // just because its expensive, but internally isDebug is already checked when - // .log() is called - String errorMessage = null; - final InputStream errorStream = connection.getErrorStream(); - if (errorStream != null) { - errorMessage = getErrorMessageFromStream(errorStream); - } + // just because its expensive, but internally isDebug is already checked when + // .log() is called + if (options.isDebug()) { + String errorMessage = getErrorMessageFromStream(connection); if (null == errorMessage || errorMessage.isEmpty()) { errorMessage = "An exception occurred while submitting the event to the Sentry server."; } - options.getLogger().log(DEBUG, errorMessage); + logger.log(DEBUG, errorMessage); } } - private String getErrorMessageFromStream(final @NotNull InputStream errorStream) { - final BufferedReader reader = new BufferedReader(new InputStreamReader(errorStream, UTF_8)); - StringBuilder sb = new StringBuilder(); - try { + /** + * Reads the error message from the error stream + * + * @param connection the HttpURLConnection + * @return the error message or null if none + */ + private @Nullable String getErrorMessageFromStream(final @NotNull HttpURLConnection connection) { + try (final InputStream errorStream = connection.getErrorStream(); + final BufferedReader reader = + new BufferedReader(new InputStreamReader(errorStream, UTF_8))) { + final StringBuilder sb = new StringBuilder(); String line; // ensure we do not add "\n" to the last line boolean first = true; @@ -480,16 +506,15 @@ private String getErrorMessageFromStream(final @NotNull InputStream errorStream) sb.append(line); first = false; } - } catch (Exception e2) { - options - .getLogger() - .log( - ERROR, - "Exception while reading the error message from the connection: " + e2.getMessage()); + return sb.toString(); + } catch (IOException e) { + logger.log(ERROR, e, "Exception while reading the error message from the connection"); } - return sb.toString(); + return null; } @Override - public void close() throws IOException {} + public void close() throws IOException { + // a connection is opened and closed for each request, so this method is not used at all. + } } diff --git a/sentry-core/src/main/java/io/sentry/core/transport/TransportResult.java b/sentry-core/src/main/java/io/sentry/core/transport/TransportResult.java index c0109b895..3ed1cf48e 100644 --- a/sentry-core/src/main/java/io/sentry/core/transport/TransportResult.java +++ b/sentry-core/src/main/java/io/sentry/core/transport/TransportResult.java @@ -29,6 +29,16 @@ public abstract class TransportResult { return new ErrorTransportResult(responseCode); } + /** + * Use this method to announce failure of sending the event. Defaults responseCode to -1 (unknown + * responseCode) + * + * @return an erroneous transport result + */ + public static @NotNull TransportResult error() { + return error(-1); + } + private TransportResult() {} public abstract boolean isSuccess(); diff --git a/sentry-core/src/test/java/io/sentry/core/transport/HttpTransportTest.kt b/sentry-core/src/test/java/io/sentry/core/transport/HttpTransportTest.kt index 7e276278a..3170befdc 100644 --- a/sentry-core/src/test/java/io/sentry/core/transport/HttpTransportTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/transport/HttpTransportTest.kt @@ -86,7 +86,7 @@ class HttpTransportTest { fun `uses Retry-After header if X-Sentry-Rate-Limit is not set when sending an event`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEventSerialize() whenever(fixture.connection.getHeaderField(eq("Retry-After"))).thenReturn("30") whenever(fixture.connection.responseCode).thenReturn(429) whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0) @@ -104,7 +104,7 @@ class HttpTransportTest { fun `uses Retry-After header if X-Sentry-Rate-Limit is not set when sending an envelope`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEnvelopeSerialize() whenever(fixture.connection.getHeaderField(eq("Retry-After"))).thenReturn("30") whenever(fixture.connection.responseCode).thenReturn(429) whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0) @@ -122,7 +122,7 @@ class HttpTransportTest { fun `passes on the response code on error when sending an event`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEventSerialize() whenever(fixture.connection.responseCode).thenReturn(1234) val event = SentryEvent() @@ -138,7 +138,7 @@ class HttpTransportTest { fun `passes on the response code on error when sending an envelope`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEnvelopeSerialize() whenever(fixture.connection.responseCode).thenReturn(1234) val envelope = SentryEnvelope.fromSession(fixture.serializer, createSession()) @@ -154,7 +154,7 @@ class HttpTransportTest { fun `uses the default retry interval if there is no Retry-After header when sending an event`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEventSerialize() whenever(fixture.connection.responseCode).thenReturn(429) whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0) @@ -171,7 +171,7 @@ class HttpTransportTest { fun `uses the default retry interval if there is no Retry-After header when sending an envelope`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEnvelopeSerialize() whenever(fixture.connection.responseCode).thenReturn(429) whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0) @@ -188,7 +188,7 @@ class HttpTransportTest { fun `failure to get response code doesn't break sending an event`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEventSerialize() whenever(fixture.connection.responseCode).thenThrow(IOException()) val event = SentryEvent() @@ -204,7 +204,7 @@ class HttpTransportTest { fun `failure to get response code doesn't break sending an envelope`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEnvelopeSerialize() whenever(fixture.connection.responseCode).thenThrow(IOException()) val session = Session("123", User(), "env", "release") @@ -221,7 +221,7 @@ class HttpTransportTest { fun `uses X-Sentry-Rate-Limit and returns accordingly`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEventSerialize() whenever(fixture.connection.getHeaderField(eq("X-Sentry-Rate-Limits"))) .thenReturn("50:transaction:key, 2700:default;error;security:organization") whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0) @@ -239,7 +239,7 @@ class HttpTransportTest { fun `uses X-Sentry-Rate-Limit and allows sending if time has passed`() { val transport = fixture.getSUT() - whenever(fixture.connection.inputStream).thenThrow(IOException()) + throwOnEventSerialize() whenever(fixture.connection.getHeaderField(eq("X-Sentry-Rate-Limits"))) .thenReturn("50:transaction:key, 1:default;error;security:organization") whenever(fixture.currentDateProvider.currentTimeMillis).thenReturn(0, 0, 1001) @@ -365,4 +365,13 @@ class HttpTransportTest { private fun createSession(): Session { return Session("123", User(), "env", "release") } + + // TODO: make inline fun , so we can throwOnSerialize() + private fun throwOnEventSerialize() { + whenever(fixture.serializer.serialize(any(), any())).thenThrow(IOException()) + } + + private fun throwOnEnvelopeSerialize() { + whenever(fixture.serializer.serialize(any(), any())).thenThrow(IOException()) + } }