From 75e8eb2fb05f6e4bb9877af9627cb67670c2192b Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Mon, 7 Oct 2024 16:25:57 +0200 Subject: [PATCH 1/8] Add retries to HTTP Get requests A quick solution to random network failures for GET HTTP requests. Should reduce the number of IOExceptions that users see while fetching data. --- build.sbt | 9 ++- .../Base/0.0.0-dev/src/Network/HTTP.enso | 2 +- .../base/enso_cloud/EnsoSecretHelper.java | 64 ++++++++++++++++++- 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/build.sbt b/build.sbt index ca40ce98b6ff..b6f2f227270d 100644 --- a/build.sbt +++ b/build.sbt @@ -502,7 +502,11 @@ val helidon = Seq( val jacksonVersion = "2.15.2" -// === JAXB ================================================================ +// == Resilience4J ============================================================ + +val resilience4JVersion = "2.2.0" + +// === JAXB =================================================================== val jaxbVersion = "4.0.0" val jaxb = Seq( @@ -4370,7 +4374,8 @@ lazy val `std-base` = project libraryDependencies ++= Seq( "org.graalvm.polyglot" % "polyglot" % graalMavenPackagesVersion, "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided", - "com.fasterxml.jackson.core" % "jackson-databind" % jacksonVersion + "com.fasterxml.jackson.core" % "jackson-databind" % jacksonVersion, + "io.github.resilience4j" % "resilience4j-retry" % resilience4JVersion ), Compile / packageBin := Def.task { val result = (Compile / packageBin).value diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso index 5d8575906eed..8471c3e6f66f 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso @@ -130,7 +130,7 @@ type HTTP all_headers = headers + boundary_header_list mapped_headers = all_headers.map on_problems=No_Wrap .to_java_pair - response = Response.Value (EnsoSecretHelper.makeRequest self.internal_http_client builder req.uri.to_java_representation mapped_headers) + response = Response.Value (EnsoSecretHelper.makeRequest self.internal_http_client builder req.uri.to_java_representation mapped_headers (req.method == HTTP_Method.Get)) if error_on_failure_code.not || response.code.is_success then response else body = response.body.decode_as_text.catch Any _->"" message = if body.is_empty then Nothing else body diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 86e9371787cc..45531826b13a 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -1,14 +1,20 @@ package org.enso.base.enso_cloud; +import io.github.resilience4j.retry.Retry; +import io.github.resilience4j.retry.RetryConfig; +import io.github.resilience4j.retry.RetryRegistry; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.net.http.HttpClient; +import java.net.http.HttpRequest; import java.net.http.HttpRequest.Builder; import java.net.http.HttpResponse; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; +import java.time.Duration; import java.util.List; import java.util.Properties; import org.enso.base.net.URISchematic; @@ -18,6 +24,32 @@ /** Makes HTTP requests with secrets in either header or query string. */ public final class EnsoSecretHelper extends SecretValueResolver { + private static class CaughtCheckedException extends RuntimeException { + private final Throwable origin; + + CaughtCheckedException(Throwable origin) { + this.origin = origin; + } + + boolean isIOException() { + return origin instanceof IOException; + } + } + + private static final RetryConfig config = + RetryConfig.custom() + .maxAttempts(3) + .waitDuration(Duration.ofMillis(100)) + .retryOnException( + e -> { + if (e instanceof CaughtCheckedException checked) return checked.isIOException(); + else return false; + }) + .build(); + + // Create a RetryRegistry with a custom global configuration + private static final RetryRegistry registry = RetryRegistry.of(config); + /** Gets a JDBC connection resolving EnsoKeyValuePair into the properties. */ public static Connection getJDBCConnection( String url, List> properties) throws SQLException { @@ -58,7 +90,8 @@ public static EnsoHttpResponse makeRequest( HttpClient client, Builder builder, URIWithSecrets uri, - List> headers) + List> headers, + Boolean withRetries) throws IllegalArgumentException, IOException, InterruptedException { // Build a new URI with the query arguments. @@ -88,8 +121,33 @@ public static EnsoHttpResponse makeRequest( // Build and Send the request. var httpRequest = builder.build(); var bodyHandler = HttpResponse.BodyHandlers.ofInputStream(); - var javaResponse = client.send(httpRequest, bodyHandler); - + Retry retry = registry.retry("request"); + var decoratedSend = + Retry.decorateFunction( + retry, + (HttpRequest request) -> { + try { + return client.send(request, bodyHandler); + } catch (Throwable e) { + throw new CaughtCheckedException(e); + } + }); + HttpResponse javaResponse; + if (withRetries) { + try { + javaResponse = decoratedSend.apply(httpRequest); + } catch (CaughtCheckedException e) { + if (e.origin instanceof IOException ioe) { + throw ioe; + } else if (e.origin instanceof InterruptedException ie) { + throw ie; + } else { + throw new IllegalStateException(e.origin); + } + } + } else { + javaResponse = client.send(httpRequest, bodyHandler); + } // Extract parts of the response return new EnsoHttpResponse( renderedURI, javaResponse.headers(), javaResponse.body(), javaResponse.statusCode()); From 5ff8ddc63a6072735a4e69aa438f5727be623f4c Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 8 Oct 2024 10:52:52 +0200 Subject: [PATCH 2/8] Use homemade retry logic for http requests --- build.sbt | 9 +-- .../base/enso_cloud/EnsoSecretHelper.java | 72 ++++++------------- 2 files changed, 22 insertions(+), 59 deletions(-) diff --git a/build.sbt b/build.sbt index b6f2f227270d..ca40ce98b6ff 100644 --- a/build.sbt +++ b/build.sbt @@ -502,11 +502,7 @@ val helidon = Seq( val jacksonVersion = "2.15.2" -// == Resilience4J ============================================================ - -val resilience4JVersion = "2.2.0" - -// === JAXB =================================================================== +// === JAXB ================================================================ val jaxbVersion = "4.0.0" val jaxb = Seq( @@ -4374,8 +4370,7 @@ lazy val `std-base` = project libraryDependencies ++= Seq( "org.graalvm.polyglot" % "polyglot" % graalMavenPackagesVersion, "org.netbeans.api" % "org-openide-util-lookup" % netbeansApiVersion % "provided", - "com.fasterxml.jackson.core" % "jackson-databind" % jacksonVersion, - "io.github.resilience4j" % "resilience4j-retry" % resilience4JVersion + "com.fasterxml.jackson.core" % "jackson-databind" % jacksonVersion ), Compile / packageBin := Def.task { val result = (Compile / packageBin).value diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 45531826b13a..58fe0463b980 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -1,20 +1,15 @@ package org.enso.base.enso_cloud; -import io.github.resilience4j.retry.Retry; -import io.github.resilience4j.retry.RetryConfig; -import io.github.resilience4j.retry.RetryRegistry; import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.net.http.HttpClient; -import java.net.http.HttpRequest; import java.net.http.HttpRequest.Builder; import java.net.http.HttpResponse; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; -import java.time.Duration; import java.util.List; import java.util.Properties; import org.enso.base.net.URISchematic; @@ -24,31 +19,7 @@ /** Makes HTTP requests with secrets in either header or query string. */ public final class EnsoSecretHelper extends SecretValueResolver { - private static class CaughtCheckedException extends RuntimeException { - private final Throwable origin; - - CaughtCheckedException(Throwable origin) { - this.origin = origin; - } - - boolean isIOException() { - return origin instanceof IOException; - } - } - - private static final RetryConfig config = - RetryConfig.custom() - .maxAttempts(3) - .waitDuration(Duration.ofMillis(100)) - .retryOnException( - e -> { - if (e instanceof CaughtCheckedException checked) return checked.isIOException(); - else return false; - }) - .build(); - - // Create a RetryRegistry with a custom global configuration - private static final RetryRegistry registry = RetryRegistry.of(config); + private static final int MAX_RETRY_ATTEMPTS = 3; /** Gets a JDBC connection resolving EnsoKeyValuePair into the properties. */ public static Connection getJDBCConnection( @@ -121,33 +92,30 @@ public static EnsoHttpResponse makeRequest( // Build and Send the request. var httpRequest = builder.build(); var bodyHandler = HttpResponse.BodyHandlers.ofInputStream(); - Retry retry = registry.retry("request"); - var decoratedSend = - Retry.decorateFunction( - retry, - (HttpRequest request) -> { - try { - return client.send(request, bodyHandler); - } catch (Throwable e) { - throw new CaughtCheckedException(e); - } - }); - HttpResponse javaResponse; - if (withRetries) { + + HttpResponse javaResponse = null; + var attempts = 0; + IOException failure = null; + while (attempts < MAX_RETRY_ATTEMPTS && javaResponse == null) { try { - javaResponse = decoratedSend.apply(httpRequest); - } catch (CaughtCheckedException e) { - if (e.origin instanceof IOException ioe) { - throw ioe; - } else if (e.origin instanceof InterruptedException ie) { - throw ie; + javaResponse = client.send(httpRequest, bodyHandler); + } catch (IOException ioe) { + if (withRetries) { + if (failure == null) { + failure = ioe; + } + attempts += 1; + Thread.sleep(Double.valueOf(Math.min(100 * Math.pow(2, attempts - 1), 5000)).longValue()); } else { - throw new IllegalStateException(e.origin); + throw ioe; } } - } else { - javaResponse = client.send(httpRequest, bodyHandler); } + if (attempts == MAX_RETRY_ATTEMPTS) { + throw failure; + } + + assert javaResponse != null; // Extract parts of the response return new EnsoHttpResponse( renderedURI, javaResponse.headers(), javaResponse.body(), javaResponse.statusCode()); From 6638f0a6fd8b7bf981f7b6b32716db7ff1a06ec7 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 8 Oct 2024 15:48:06 +0200 Subject: [PATCH 3/8] Add retries to whole Data.read Previously, we added retries only to fetch HTTP_Request. That was insufficient as intermittent errors might happen while reading body's stream. Enhanced our simple server's crash endpoint to allow for different kind of failures as well as simulate random failures. --- .../src/Internal/Data_Read_Helpers.enso | 23 +++++- .../Base/0.0.0-dev/src/Runtime/Thread.enso | 16 +++++ .../expression/builtin/thread/SleepNode.java | 52 ++++++++++++++ test/Base_Tests/src/Network/Http_Spec.enso | 18 +++-- .../test_helpers/CrashingTestHandler.java | 72 ++++++++++++++++++- 5 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso index 0bf4b570ddf0..f6cd14f6effd 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso @@ -5,13 +5,18 @@ import project.Data.Text.Text import project.Data.Vector.Vector import project.Enso_Cloud.Data_Link.Data_Link import project.Enso_Cloud.Data_Link_Helpers +import project.Error.Error import project.Errors.Deprecated.Deprecated import project.Errors.Problem_Behavior.Problem_Behavior +import project.IO +import project.Math import project.Metadata.Display import project.Metadata.Widget import project.Network.HTTP.HTTP +import project.Network.HTTP.HTTP_Error.HTTP_Error import project.Network.HTTP.HTTP_Method.HTTP_Method import project.Network.URI.URI +import project.Runtime.Thread import project.Warning.Warning from project.Data import Raw_Response from project.Data.Boolean import Boolean, False, True @@ -28,8 +33,22 @@ looks_like_uri path:Text -> Boolean = A common implementation for fetching a resource and decoding it, following encountered data links. fetch_following_data_links (uri:URI) (method:HTTP_Method = HTTP_Method.Get) (headers:Vector = []) format = - response = HTTP.fetch uri method headers - decode_http_response_following_data_links response format + fetch_and_decode = + response = HTTP.fetch uri method headers + decode_http_response_following_data_links response format + + error_handler attempt = + caught_error -> + case method of + HTTP_Method.Get -> + if attempt > 2 then Error.throw caught_error else + sleep_time = Math.min (100 * (2 ^ (attempt - 1))) 5000 + Thread.sleep sleep_time + fetch_and_decode . catch HTTP_Error (error_handler (attempt + 1)) + _ -> + Error.throw caught_error.payload + + fetch_and_decode . catch HTTP_Error (error_handler 0) ## PRIVATE Decodes a HTTP response, handling data link access. diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso index 50d858cc3476..0f3441a1b7c1 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso @@ -2,6 +2,7 @@ Internal threading utilities used for working with threads. import project.Any.Any +import project.Data.Numbers.Integer ## PRIVATE ADVANCED @@ -19,3 +20,18 @@ import project.Any.Any Thread.with_interrupt_handler (1 + 1) <| IO.println "I died!" with_interrupt_handler : Any -> Any -> Any with_interrupt_handler ~action ~interrupt_handler = @Builtin_Method "Thread.with_interrupt_handler" + +## PRIVATE + ADVANCED + Temporarily cease execution of the current thread. + + Arguments: + - time: amount of milliseconds to sleep + + > Example + Sleep the current thread for 1 second. + + Thread.sleep 1000 <| IO.println "I continue!" +sleep : Integer +sleep time_in_milliseconds = @Builtin_Method "Thread.sleep" + diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java new file mode 100644 index 000000000000..e65bdf97cde6 --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java @@ -0,0 +1,52 @@ +package org.enso.interpreter.node.expression.builtin.thread; + +import com.oracle.truffle.api.dsl.Fallback; +import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.nodes.Node; +import org.enso.interpreter.dsl.BuiltinMethod; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.control.ThreadInterruptedException; +import org.enso.interpreter.runtime.error.PanicException; + +@BuiltinMethod( + type = "Thread", + name = "sleep", + description = "Sleep in the current thread.", + autoRegister = false, + inlineable = true) +public abstract class SleepNode extends Node { + + public abstract Object execute(Object timeInMilliseconds); + + public static SleepNode build() { + return SleepNodeGen.create(); + } + + @Specialization + Object doLong(long timeInMilliseconds) { + try { + Thread.sleep(timeInMilliseconds); + return EnsoContext.get(this).getBuiltins().nothing(); + } catch (InterruptedException e) { + throw new ThreadInterruptedException(); + } + } + + @Specialization + Object doDouble(double timeInMilliseconds) { + try { + Thread.sleep(Double.valueOf(timeInMilliseconds).longValue()); + return EnsoContext.get(this).getBuiltins().nothing(); + } catch (InterruptedException e) { + throw new ThreadInterruptedException(); + } + } + + @Fallback + Object doOther(Object timeInMilliseconds) { + var builtins = EnsoContext.get(this).getBuiltins(); + var intType = builtins.number().getInteger(); + throw new PanicException( + builtins.error().makeTypeError(intType, timeInMilliseconds, "timef"), this); + } +} diff --git a/test/Base_Tests/src/Network/Http_Spec.enso b/test/Base_Tests/src/Network/Http_Spec.enso index b005e8dba67c..c3a03d5a8c15 100644 --- a/test/Base_Tests/src/Network/Http_Spec.enso +++ b/test/Base_Tests/src/Network/Http_Spec.enso @@ -591,7 +591,7 @@ add_specs suite_builder = ## Checking this error partially as a warning - I spent a lot of time debugging why I'm getting such an error. Apparently it happens when the httpbin server was crashing without sending any response. - group_builder.specify "should be able to handle server crash resulting in no response" pending=pending_has_url <| Test.with_retries <| + group_builder.specify "should be able to handle server crash resulting in no response" pending=pending_has_url <| err = Data.fetch (base_url_with_slash+"crash") err.should_fail_with Request_Error err.catch.error_type . should_equal "java.io.IOException" @@ -600,10 +600,20 @@ add_specs suite_builder = I think it may be worth adding, because it may be really quite confusing for end users who get that kind of error. err.catch.message . should_equal "HTTP/1.1 header parser received no bytes" - group_builder.specify "should be able to handle IO errors" pending="TODO: Currently I was unable to figure out a way to test such errors" <| - # how to trigger this error??? - err = Data.fetch "TODO" + group_builder.specify "should be able to handle occasional server crashes and retry" pending=pending_has_url <| + r1 = Data.fetch (base_url_with_slash+"crash?success_every=2") + r1.should_succeed + r1.should_be_a Response + + group_builder.specify "should be able to handle server crash that closes stream abruptly" pending=pending_has_url <| + err = Data.fetch (base_url_with_slash+"crash?type=stream") err.should_fail_with HTTP_Error + err.catch.message . should_equal "An IO error has occurred: java.io.IOException: closed" + + group_builder.specify "should be able to handle occasional abrupt stream closures and retry" pending=pending_has_url <| + r1 = Data.fetch (base_url_with_slash+"crash?type=stream&success_every=2") + r1.should_succeed + r1.should_be_a Response suite_builder.group "Http Auth" group_builder-> group_builder.specify "should support Basic user+password authentication" pending=pending_has_url <| Test.with_retries <| diff --git a/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java b/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java index 88bbea1bb006..00eaaad3a517 100644 --- a/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java +++ b/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java @@ -2,13 +2,81 @@ import com.sun.net.httpserver.HttpExchange; import java.io.IOException; +import java.io.OutputStream; +import java.net.URI; +import org.apache.http.client.utils.URIBuilder; import org.enso.shttp.SimpleHttpHandler; public class CrashingTestHandler extends SimpleHttpHandler { + private int requests = 0; + @Override protected void doHandle(HttpExchange exchange) throws IOException { - // This exception will be logged by SimpleHttpHandler, but that's OK - let's know that this + // Exceptions will be logged by SimpleHttpHandler, but that's OK - let's know that this // crash is happening. - throw new RuntimeException("This handler crashes on purpose."); + + URI uri = exchange.getRequestURI(); + URIBuilder builder = new URIBuilder(uri); + final String successEveryParam = "success_every"; + final String crashTypeParam = "type"; + + int successEvery = 0; + CrashType crashType = CrashType.RUNTIME; + + for (var queryPair : builder.getQueryParams()) { + if (queryPair.getName().equals(successEveryParam)) { + successEvery = Integer.decode(queryPair.getValue()); + } else if (queryPair.getName().equals(crashTypeParam)) { + crashType = + switch (queryPair.getValue()) { + case "stream" -> CrashType.STREAM; + default -> CrashType.RUNTIME; + }; + } + } + if (successEvery == 0) { + // Reset counter + requests = 0; + } + + switch (crashType) { + case RUNTIME: + if (successEvery == (requests + 1)) { + // return OK, reset + requests = 0; + exchange.sendResponseHeaders(200, -1); + exchange.close(); + break; + } else { + requests += 1; + throw new RuntimeException("This handler crashes on purpose."); + } + + case STREAM: + byte[] responseData = "Crash and Burn".getBytes(); + exchange.sendResponseHeaders(200, responseData.length); + try { + if (successEvery == (requests + 1)) { + requests = 0; + // return OK, reset + try (OutputStream os = exchange.getResponseBody()) { + os.write(responseData, 0, responseData.length); + } + } else { + requests += 1; + try (OutputStream os = exchange.getResponseBody()) { + os.write(responseData, 0, responseData.length / 2); + } + } + } finally { + exchange.close(); + } + break; + } + } + + enum CrashType { + RUNTIME, + STREAM } } From b1de98287f4e1744e11dd9df920a46573b065eeb Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 8 Oct 2024 15:56:36 +0200 Subject: [PATCH 4/8] Remove retries from Java Increased the scope of retries in the previous commit. --- .../Base/0.0.0-dev/src/Network/HTTP.enso | 2 +- .../base/enso_cloud/EnsoSecretHelper.java | 30 ++----------------- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso index 8471c3e6f66f..5d8575906eed 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP.enso @@ -130,7 +130,7 @@ type HTTP all_headers = headers + boundary_header_list mapped_headers = all_headers.map on_problems=No_Wrap .to_java_pair - response = Response.Value (EnsoSecretHelper.makeRequest self.internal_http_client builder req.uri.to_java_representation mapped_headers (req.method == HTTP_Method.Get)) + response = Response.Value (EnsoSecretHelper.makeRequest self.internal_http_client builder req.uri.to_java_representation mapped_headers) if error_on_failure_code.not || response.code.is_success then response else body = response.body.decode_as_text.catch Any _->"" message = if body.is_empty then Nothing else body diff --git a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java index 58fe0463b980..86e9371787cc 100644 --- a/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java +++ b/std-bits/base/src/main/java/org/enso/base/enso_cloud/EnsoSecretHelper.java @@ -1,7 +1,6 @@ package org.enso.base.enso_cloud; import java.io.IOException; -import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.net.http.HttpClient; @@ -19,8 +18,6 @@ /** Makes HTTP requests with secrets in either header or query string. */ public final class EnsoSecretHelper extends SecretValueResolver { - private static final int MAX_RETRY_ATTEMPTS = 3; - /** Gets a JDBC connection resolving EnsoKeyValuePair into the properties. */ public static Connection getJDBCConnection( String url, List> properties) throws SQLException { @@ -61,8 +58,7 @@ public static EnsoHttpResponse makeRequest( HttpClient client, Builder builder, URIWithSecrets uri, - List> headers, - Boolean withRetries) + List> headers) throws IllegalArgumentException, IOException, InterruptedException { // Build a new URI with the query arguments. @@ -92,30 +88,8 @@ public static EnsoHttpResponse makeRequest( // Build and Send the request. var httpRequest = builder.build(); var bodyHandler = HttpResponse.BodyHandlers.ofInputStream(); + var javaResponse = client.send(httpRequest, bodyHandler); - HttpResponse javaResponse = null; - var attempts = 0; - IOException failure = null; - while (attempts < MAX_RETRY_ATTEMPTS && javaResponse == null) { - try { - javaResponse = client.send(httpRequest, bodyHandler); - } catch (IOException ioe) { - if (withRetries) { - if (failure == null) { - failure = ioe; - } - attempts += 1; - Thread.sleep(Double.valueOf(Math.min(100 * Math.pow(2, attempts - 1), 5000)).longValue()); - } else { - throw ioe; - } - } - } - if (attempts == MAX_RETRY_ATTEMPTS) { - throw failure; - } - - assert javaResponse != null; // Extract parts of the response return new EnsoHttpResponse( renderedURI, javaResponse.headers(), javaResponse.body(), javaResponse.statusCode()); From f9cc12336171001a2482b1247419f0614a00a0d0 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 8 Oct 2024 16:04:48 +0200 Subject: [PATCH 5/8] nit --- .../interpreter/node/expression/builtin/thread/SleepNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java index e65bdf97cde6..0204c607d4c9 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java @@ -47,6 +47,6 @@ Object doOther(Object timeInMilliseconds) { var builtins = EnsoContext.get(this).getBuiltins(); var intType = builtins.number().getInteger(); throw new PanicException( - builtins.error().makeTypeError(intType, timeInMilliseconds, "timef"), this); + builtins.error().makeTypeError(intType, timeInMilliseconds, "timeInMilliseconds"), this); } } From 43f6b1d2f4300d83a87c7fec88d6a44f1fc065f6 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 8 Oct 2024 17:41:21 +0200 Subject: [PATCH 6/8] Address PR comments --- .../0.0.0-dev/src/Internal/Data_Read_Helpers.enso | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso index f6cd14f6effd..9b84f464d744 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso @@ -8,8 +8,6 @@ import project.Enso_Cloud.Data_Link_Helpers import project.Error.Error import project.Errors.Deprecated.Deprecated import project.Errors.Problem_Behavior.Problem_Behavior -import project.IO -import project.Math import project.Metadata.Display import project.Metadata.Widget import project.Network.HTTP.HTTP @@ -39,14 +37,10 @@ fetch_following_data_links (uri:URI) (method:HTTP_Method = HTTP_Method.Get) (hea error_handler attempt = caught_error -> - case method of - HTTP_Method.Get -> - if attempt > 2 then Error.throw caught_error else - sleep_time = Math.min (100 * (2 ^ (attempt - 1))) 5000 - Thread.sleep sleep_time - fetch_and_decode . catch HTTP_Error (error_handler (attempt + 1)) - _ -> - Error.throw caught_error.payload + if method != HTTP_Method.Get || attempt > 2 then Error.throw caught_error else + sleep_time = [100, 200, 400] . get attempt + Thread.sleep sleep_time + fetch_and_decode . catch HTTP_Error (error_handler (attempt + 1)) fetch_and_decode . catch HTTP_Error (error_handler 0) From a02492e3f32616fbd56511f50807ed7a8e6b65ef Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 9 Oct 2024 11:18:14 +0200 Subject: [PATCH 7/8] PR comments --- .../Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso | 5 +++-- .../Standard/Base/0.0.0-dev/src/Runtime/Thread.enso | 4 ++-- .../enso/shttp/test_helpers/CrashingTestHandler.java | 10 ++++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso index 9b84f464d744..aa3607b8d071 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso @@ -37,8 +37,9 @@ fetch_following_data_links (uri:URI) (method:HTTP_Method = HTTP_Method.Get) (hea error_handler attempt = caught_error -> - if method != HTTP_Method.Get || attempt > 2 then Error.throw caught_error else - sleep_time = [100, 200, 400] . get attempt + exponential_backoff = [100, 200, 400] + if method != HTTP_Method.Get || attempt >= exponential_backoff.length then Error.throw caught_error else + sleep_time = exponential_backoff . get attempt Thread.sleep sleep_time fetch_and_decode . catch HTTP_Error (error_handler (attempt + 1)) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso index 0f3441a1b7c1..291a982c0665 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso @@ -19,7 +19,7 @@ import project.Data.Numbers.Integer Thread.with_interrupt_handler (1 + 1) <| IO.println "I died!" with_interrupt_handler : Any -> Any -> Any -with_interrupt_handler ~action ~interrupt_handler = @Builtin_Method "Thread.with_interrupt_handler" +private with_interrupt_handler ~action ~interrupt_handler = @Builtin_Method "Thread.with_interrupt_handler" ## PRIVATE ADVANCED @@ -33,5 +33,5 @@ with_interrupt_handler ~action ~interrupt_handler = @Builtin_Method "Thread.with Thread.sleep 1000 <| IO.println "I continue!" sleep : Integer -sleep time_in_milliseconds = @Builtin_Method "Thread.sleep" +private sleep time_in_milliseconds = @Builtin_Method "Thread.sleep" diff --git a/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java b/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java index 00eaaad3a517..b43d51d8c973 100644 --- a/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java +++ b/tools/http-test-helper/src/main/java/org/enso/shttp/test_helpers/CrashingTestHandler.java @@ -39,9 +39,11 @@ protected void doHandle(HttpExchange exchange) throws IOException { requests = 0; } + boolean shouldSucceed = successEvery == (requests + 1); + switch (crashType) { case RUNTIME: - if (successEvery == (requests + 1)) { + if (shouldSucceed) { // return OK, reset requests = 0; exchange.sendResponseHeaders(200, -1); @@ -56,7 +58,7 @@ protected void doHandle(HttpExchange exchange) throws IOException { byte[] responseData = "Crash and Burn".getBytes(); exchange.sendResponseHeaders(200, responseData.length); try { - if (successEvery == (requests + 1)) { + if (shouldSucceed) { requests = 0; // return OK, reset try (OutputStream os = exchange.getResponseBody()) { @@ -76,7 +78,7 @@ protected void doHandle(HttpExchange exchange) throws IOException { } enum CrashType { - RUNTIME, - STREAM + RUNTIME, // Simulate an internal server crash + STREAM // Simulate a crash by abruptly closing response's body stream } } From d408d7c7726bb3f32a44df73113513a0daf7b143 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Wed, 9 Oct 2024 14:46:10 +0200 Subject: [PATCH 8/8] Remove builtin --- .../src/Internal/Data_Read_Helpers.enso | 3 +- .../Base/0.0.0-dev/src/Runtime/Thread.enso | 17 +----- .../expression/builtin/thread/SleepNode.java | 52 ------------------- 3 files changed, 3 insertions(+), 69 deletions(-) delete mode 100644 engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso index aa3607b8d071..37d879f0a493 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Data_Read_Helpers.enso @@ -14,7 +14,6 @@ import project.Network.HTTP.HTTP import project.Network.HTTP.HTTP_Error.HTTP_Error import project.Network.HTTP.HTTP_Method.HTTP_Method import project.Network.URI.URI -import project.Runtime.Thread import project.Warning.Warning from project.Data import Raw_Response from project.Data.Boolean import Boolean, False, True @@ -23,6 +22,8 @@ from project.Metadata.Choice import Option from project.Metadata.Widget import Single_Choice from project.System.File_Format import Auto_Detect, format_types +polyglot java import java.lang.Thread + ## PRIVATE looks_like_uri path:Text -> Boolean = (path.starts_with "http://" Case_Sensitivity.Insensitive) || (path.starts_with "https://" Case_Sensitivity.Insensitive) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso index 291a982c0665..861c4405225b 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Runtime/Thread.enso @@ -19,19 +19,4 @@ import project.Data.Numbers.Integer Thread.with_interrupt_handler (1 + 1) <| IO.println "I died!" with_interrupt_handler : Any -> Any -> Any -private with_interrupt_handler ~action ~interrupt_handler = @Builtin_Method "Thread.with_interrupt_handler" - -## PRIVATE - ADVANCED - Temporarily cease execution of the current thread. - - Arguments: - - time: amount of milliseconds to sleep - - > Example - Sleep the current thread for 1 second. - - Thread.sleep 1000 <| IO.println "I continue!" -sleep : Integer -private sleep time_in_milliseconds = @Builtin_Method "Thread.sleep" - +with_interrupt_handler ~action ~interrupt_handler = @Builtin_Method "Thread.with_interrupt_handler" diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java deleted file mode 100644 index 0204c607d4c9..000000000000 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/thread/SleepNode.java +++ /dev/null @@ -1,52 +0,0 @@ -package org.enso.interpreter.node.expression.builtin.thread; - -import com.oracle.truffle.api.dsl.Fallback; -import com.oracle.truffle.api.dsl.Specialization; -import com.oracle.truffle.api.nodes.Node; -import org.enso.interpreter.dsl.BuiltinMethod; -import org.enso.interpreter.runtime.EnsoContext; -import org.enso.interpreter.runtime.control.ThreadInterruptedException; -import org.enso.interpreter.runtime.error.PanicException; - -@BuiltinMethod( - type = "Thread", - name = "sleep", - description = "Sleep in the current thread.", - autoRegister = false, - inlineable = true) -public abstract class SleepNode extends Node { - - public abstract Object execute(Object timeInMilliseconds); - - public static SleepNode build() { - return SleepNodeGen.create(); - } - - @Specialization - Object doLong(long timeInMilliseconds) { - try { - Thread.sleep(timeInMilliseconds); - return EnsoContext.get(this).getBuiltins().nothing(); - } catch (InterruptedException e) { - throw new ThreadInterruptedException(); - } - } - - @Specialization - Object doDouble(double timeInMilliseconds) { - try { - Thread.sleep(Double.valueOf(timeInMilliseconds).longValue()); - return EnsoContext.get(this).getBuiltins().nothing(); - } catch (InterruptedException e) { - throw new ThreadInterruptedException(); - } - } - - @Fallback - Object doOther(Object timeInMilliseconds) { - var builtins = EnsoContext.get(this).getBuiltins(); - var intType = builtins.number().getInteger(); - throw new PanicException( - builtins.error().makeTypeError(intType, timeInMilliseconds, "timeInMilliseconds"), this); - } -}