feat: enhance proxy exceptions (#837)

Author: ianbotsf

.changes/2c77890c-7867-41d2-b0c4-6e28525989d3.json

@@ -0,0 +1,5 @@
+{
+    "id": "2c77890c-7867-41d2-b0c4-6e28525989d3",
+    "type": "feature",
+    "description": "Enhance exceptions thrown during proxy config parsing"
+}

runtime/protocol/http-client/common/src/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelector.kt

@@ -5,6 +5,7 @@
 
 package aws.smithy.kotlin.runtime.http.engine
 
+import aws.smithy.kotlin.runtime.ClientException
 import aws.smithy.kotlin.runtime.net.Host
 import aws.smithy.kotlin.runtime.net.Scheme
 import aws.smithy.kotlin.runtime.net.Url
@@ -58,17 +59,35 @@ private fun resolveProxyByProperty(provider: PropertyProvider, scheme: Scheme):
         // we don't support connecting to the proxy over TLS, we expect engines would support
         // tunneling https traffic via HTTP Connect to the proxy
         val proxyProtocol = Scheme.HTTP
-        val url = Url(proxyProtocol, Host.parse(hostName), proxyPortProp?.toInt() ?: scheme.defaultPort)
+
+        val url = try {
+            Url(proxyProtocol, Host.parse(hostName), proxyPortProp?.toInt() ?: scheme.defaultPort)
+        } catch (e: Exception) {
+            val parsed = buildString {
+                append("""$hostPropName="$proxyHostProp"""")
+                proxyPortProp?.let { append(""", $hostPortPropName="$it"""") }
+            }
+            throw ClientException("Could not parse $parsed into a valid proxy URL", e)
+        }
+
         ProxyConfig.Http(url)
     }
 }
 
 private fun resolveProxyByEnvironment(provider: EnvironmentProvider, scheme: Scheme): ProxyConfig? =
     // lowercase takes precedence: https://about.gitlab.com/blog/2021/01/27/we-need-to-talk-no-proxy/
     listOf("${scheme.protocolName.lowercase()}_proxy", "${scheme.protocolName.uppercase()}_PROXY")
-        .mapNotNull { provider.getenv(it) }
-        .map { ProxyConfig.Http(Url.parse(it)) }
-        .firstOrNull()
+        .firstNotNullOfOrNull { envVar ->
+            provider.getenv(envVar)?.let { proxyUrlString ->
+                val url = try {
+                    Url.parse(proxyUrlString)
+                } catch (e: Exception) {
+                    val parsed = """$envVar="$proxyUrlString""""
+                    throw ClientException("Could not parse $parsed into a valid proxy URL", e)
+                }
+                ProxyConfig.Http(url)
+            }
+        }
 
 internal data class NoProxyHost(val hostMatch: String, val port: Int? = null) {
     fun matches(url: Url): Boolean {

runtime/protocol/http-client/common/test/aws/smithy/kotlin/runtime/http/engine/EnvironmentProxySelectorTest.kt

@@ -5,9 +5,12 @@
 
 package aws.smithy.kotlin.runtime.http.engine
 
+import aws.smithy.kotlin.runtime.ClientException
 import aws.smithy.kotlin.runtime.net.Url
 import aws.smithy.kotlin.runtime.util.PlatformEnvironProvider
+import org.junit.jupiter.api.assertThrows
 import kotlin.test.Test
+import kotlin.test.assertContains
 import kotlin.test.assertEquals
 
 data class TestPlatformEnvironmentProvider(
@@ -77,4 +80,39 @@ class EnvironmentProxySelectorTest {
             assertEquals(testCase.expected, actual, "[idx=$idx] expected $testCase resulted in proxy config: $actual")
         }
     }
+
+    private data class FailCase(
+        val env: Map<String, String> = emptyMap(),
+        val props: Map<String, String> = emptyMap(),
+    )
+
+    private val failCases = listOf(
+        // Invalid ports specified
+        FailCase(props = mapOf("http.proxyHost" to "test.proxy.aws", "http.proxyPort" to "0")),
+        FailCase(props = mapOf("http.proxyHost" to "test.proxy.aws", "http.proxyPort" to "x")),
+        FailCase(props = mapOf("https.proxyHost" to "test.proxy.aws", "https.proxyPort" to "0")),
+        FailCase(props = mapOf("https.proxyHost" to "test.proxy.aws", "https.proxyPort" to "x")),
+        FailCase(env = mapOf("http_proxy" to "http://test.proxy.aws:0")),
+        FailCase(env = mapOf("http_proxy" to "http://test.proxy.aws:x")),
+        FailCase(env = mapOf("https_proxy" to "https://test.proxy.aws:0")),
+        FailCase(env = mapOf("https_proxy" to "https://test.proxy.aws:x")),
+    )
+
+    @Test
+    fun testSelectFailures() {
+        failCases.forEachIndexed { idx, failCase ->
+            val testProvider = TestPlatformEnvironmentProvider(failCase.env, failCase.props)
+            val exception = assertThrows<ClientException>("[idx=$idx] expected ClientException") {
+                EnvironmentProxySelector(testProvider)
+            }
+
+            val expectedError = (failCase.env + failCase.props).map { (k, v) -> """$k="$v"""" }.joinToString(", ")
+
+            assertContains(
+                exception.message!!,
+                expectedError,
+                message = "[idx=$idx] unexpected error message ${exception.message}",
+            )
+        }
+    }
 }

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/net/Url.kt

@@ -35,7 +35,7 @@ public data class Url(
     public val encodeParameters: Boolean = true,
 ) {
     init {
-        require(port in 1..65535) { "port must be in range [1, 65535]" }
+        require(port in 1..65535) { "Given port $port is not in required range [1, 65535]" }
     }
 
     public companion object {