mirror of
https://github.com/lukaszraczylo/go-simple-graphql.git
synced 2026-10-13 00:48:37 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cd16b92dca |
+11
-23
@@ -3,7 +3,6 @@ package gql
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"compress/gzip"
|
"compress/gzip"
|
||||||
"crypto/tls"
|
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
@@ -74,23 +73,8 @@ func isRetryableError(errors []struct {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
var (
|
// Note: defaultClient removed - all clients should be created via NewConnection()
|
||||||
// Shared HTTP transport with optimized settings
|
// which properly configures HTTP/2 transport with correct settings
|
||||||
defaultTransport = &http.Transport{
|
|
||||||
MaxIdleConns: 100,
|
|
||||||
MaxIdleConnsPerHost: 100,
|
|
||||||
IdleConnTimeout: 90 * time.Second,
|
|
||||||
DisableCompression: true, // Disable automatic request compression to prevent "trailing garbage" errors
|
|
||||||
ForceAttemptHTTP2: true,
|
|
||||||
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // Skip TLS verification for test environments
|
|
||||||
}
|
|
||||||
|
|
||||||
// Shared HTTP client with timeouts
|
|
||||||
defaultClient = &http.Client{
|
|
||||||
Transport: defaultTransport,
|
|
||||||
Timeout: 30 * time.Second,
|
|
||||||
}
|
|
||||||
)
|
|
||||||
|
|
||||||
func (qe *QueryExecutor) executeQuery() ([]byte, error) {
|
func (qe *QueryExecutor) executeQuery() ([]byte, error) {
|
||||||
// Reuse buffer from pool to avoid allocations
|
// Reuse buffer from pool to avoid allocations
|
||||||
@@ -173,12 +157,16 @@ func (qe *QueryExecutor) executeQuery() ([]byte, error) {
|
|||||||
},
|
},
|
||||||
})
|
})
|
||||||
|
|
||||||
// Use default client if custom client is not set
|
// Client should always be set via NewConnection(), but check for safety
|
||||||
client := qe.client
|
if qe.client == nil {
|
||||||
if client == nil {
|
qe.Logger.Error(&libpack_logger.LogMessage{
|
||||||
client = defaultClient
|
Message: "HTTP client not initialized - use NewConnection() to create client",
|
||||||
|
Pairs: nil,
|
||||||
|
})
|
||||||
|
return fmt.Errorf("HTTP client not initialized")
|
||||||
}
|
}
|
||||||
httpResponse, err := client.Do(httpRequest)
|
|
||||||
|
httpResponse, err := qe.client.Do(httpRequest)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -185,7 +185,7 @@ func (suite *Tests) TestQueryExecutor_executeQuery() {
|
|||||||
assert.Equal(3, attempts) // Should have retried twice before succeeding
|
assert.Equal(3, attempts) // Should have retried twice before succeeding
|
||||||
})
|
})
|
||||||
|
|
||||||
suite.T().Run("should use default client when none provided", func(t *testing.T) {
|
suite.T().Run("should return error when client not initialized", func(t *testing.T) {
|
||||||
client := CreateTestClient()
|
client := CreateTestClient()
|
||||||
|
|
||||||
qe := &QueryExecutor{
|
qe := &QueryExecutor{
|
||||||
@@ -196,12 +196,13 @@ func (suite *Tests) TestQueryExecutor_executeQuery() {
|
|||||||
Retries: false,
|
Retries: false,
|
||||||
}
|
}
|
||||||
qe.endpoint = mockServer.URL
|
qe.endpoint = mockServer.URL
|
||||||
qe.client = nil // No client provided
|
qe.client = nil // No client provided (should always use NewConnection())
|
||||||
qe.cache = client.cache
|
qe.cache = client.cache
|
||||||
|
|
||||||
result, err := qe.executeQuery()
|
result, err := qe.executeQuery()
|
||||||
assert.NoError(err)
|
assert.Error(err)
|
||||||
assert.NotNil(result)
|
assert.Nil(result)
|
||||||
|
assert.Contains(err.Error(), "HTTP client not initialized")
|
||||||
})
|
})
|
||||||
|
|
||||||
suite.T().Run("should set default Content-Type header", func(t *testing.T) {
|
suite.T().Run("should set default Content-Type header", func(t *testing.T) {
|
||||||
|
|||||||
@@ -11,60 +11,50 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
func (b *BaseClient) createHttpClient() (http_client *http.Client) {
|
func (b *BaseClient) createHttpClient() (http_client *http.Client) {
|
||||||
// Optimized connection pool settings for better performance
|
// Create TLS config for HTTPS endpoints
|
||||||
httpTransport := &http.Transport{
|
var tlsClientConfig *tls.Config
|
||||||
MaxIdleConns: 100, // Reduced from 512
|
if strings.HasPrefix(b.endpoint, "https://") {
|
||||||
MaxConnsPerHost: 50, // Reduced from 512
|
tlsClientConfig = &tls.Config{
|
||||||
MaxIdleConnsPerHost: 10, // Reduced from 512
|
InsecureSkipVerify: true, // TODO: Make this configurable via environment variable
|
||||||
IdleConnTimeout: 30 * time.Second, // Increased for better reuse
|
}
|
||||||
ResponseHeaderTimeout: 10 * time.Second, // Reduced for faster timeouts
|
|
||||||
DisableKeepAlives: false,
|
|
||||||
DisableCompression: true, // Disable automatic request compression to prevent "trailing garbage" errors
|
|
||||||
WriteBufferSize: 4096, // Optimize buffer size
|
|
||||||
ReadBufferSize: 4096, // Optimize buffer size
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Use HTTP/2 transport for both http:// (h2c) and https:// endpoints
|
||||||
|
// AllowHTTP=true enables HTTP/2 Cleartext (h2c) for unencrypted connections
|
||||||
|
http2Transport := &http2.Transport{
|
||||||
|
AllowHTTP: true, // Enable h2c for http:// endpoints
|
||||||
|
TLSClientConfig: tlsClientConfig, // nil for http://, configured for https://
|
||||||
|
ReadIdleTimeout: 30 * time.Second, // Increased for better connection reuse
|
||||||
|
PingTimeout: 10 * time.Second, // Reduced for faster dead connection detection
|
||||||
|
WriteByteTimeout: 10 * time.Second, // Write timeout for better error handling
|
||||||
|
DisableCompression: true, // Disable automatic compression to prevent trailing garbage errors
|
||||||
|
}
|
||||||
|
|
||||||
|
http_client = &http.Client{
|
||||||
|
Timeout: 30 * time.Second, // Overall request timeout
|
||||||
|
Transport: http2Transport,
|
||||||
|
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
||||||
|
return http.ErrUseLastResponse // Don't follow redirects automatically
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// Log which protocol is being used
|
||||||
if strings.HasPrefix(b.endpoint, "http://") {
|
if strings.HasPrefix(b.endpoint, "http://") {
|
||||||
http_client = &http.Client{
|
|
||||||
Timeout: 30 * time.Second, // Increased for better reliability
|
|
||||||
Transport: httpTransport,
|
|
||||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
|
||||||
return http.ErrUseLastResponse
|
|
||||||
},
|
|
||||||
}
|
|
||||||
b.Logger.Debug(&libpack_logger.LogMessage{
|
b.Logger.Debug(&libpack_logger.LogMessage{
|
||||||
Message: "Using HTTP/1.1 over http",
|
Message: "Using HTTP/2 Cleartext (h2c) over http",
|
||||||
Pairs: nil,
|
Pairs: nil,
|
||||||
})
|
})
|
||||||
} else if strings.HasPrefix(b.endpoint, "https://") {
|
} else if strings.HasPrefix(b.endpoint, "https://") {
|
||||||
tlsClientConfig := &tls.Config{}
|
|
||||||
if strings.HasPrefix(b.endpoint, "https://") {
|
|
||||||
tlsClientConfig.InsecureSkipVerify = true
|
|
||||||
}
|
|
||||||
http2Transport := &http2.Transport{
|
|
||||||
AllowHTTP: true,
|
|
||||||
TLSClientConfig: tlsClientConfig,
|
|
||||||
ReadIdleTimeout: 30 * time.Second, // Increased for better reuse
|
|
||||||
PingTimeout: 10 * time.Second, // Reduced for faster detection
|
|
||||||
WriteByteTimeout: 10 * time.Second, // Add write timeout
|
|
||||||
DisableCompression: true, // Disable automatic request compression to prevent "trailing garbage" errors
|
|
||||||
}
|
|
||||||
http_client = &http.Client{
|
|
||||||
Timeout: 30 * time.Second, // Increased for better reliability
|
|
||||||
Transport: http2Transport,
|
|
||||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
|
||||||
return http.ErrUseLastResponse
|
|
||||||
},
|
|
||||||
}
|
|
||||||
b.Logger.Debug(&libpack_logger.LogMessage{
|
b.Logger.Debug(&libpack_logger.LogMessage{
|
||||||
Message: "Using HTTP/2 over https",
|
Message: "Using HTTP/2 over TLS (https)",
|
||||||
Pairs: nil,
|
Pairs: nil,
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
b.Logger.Critical(&libpack_logger.LogMessage{
|
b.Logger.Critical(&libpack_logger.LogMessage{
|
||||||
Message: "Invalid endpoint - neither http or https",
|
Message: "Invalid endpoint - must start with http:// or https://",
|
||||||
})
|
})
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return http_client
|
return http_client
|
||||||
}
|
}
|
||||||
|
|||||||
+19
-23
@@ -8,8 +8,10 @@ import (
|
|||||||
"golang.org/x/net/http2"
|
"golang.org/x/net/http2"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// Note: http.Transport import is kept for http.ErrUseLastResponse usage in redirect tests
|
||||||
|
|
||||||
func (suite *Tests) TestBaseClient_createHttpClient() {
|
func (suite *Tests) TestBaseClient_createHttpClient() {
|
||||||
suite.T().Run("should create HTTP/1.1 client for http endpoints", func(t *testing.T) {
|
suite.T().Run("should create HTTP/2 client for http endpoints (h2c)", func(t *testing.T) {
|
||||||
client := NewConnection()
|
client := NewConnection()
|
||||||
client.SetEndpoint("http://example.com/graphql")
|
client.SetEndpoint("http://example.com/graphql")
|
||||||
|
|
||||||
@@ -18,18 +20,15 @@ func (suite *Tests) TestBaseClient_createHttpClient() {
|
|||||||
assert.Equal(30*time.Second, httpClient.Timeout)
|
assert.Equal(30*time.Second, httpClient.Timeout)
|
||||||
assert.NotNil(httpClient.Transport)
|
assert.NotNil(httpClient.Transport)
|
||||||
|
|
||||||
// Check that it's using regular HTTP transport
|
// Check that it's using HTTP/2 transport with h2c (HTTP/2 Cleartext)
|
||||||
transport, ok := httpClient.Transport.(*http.Transport)
|
transport, ok := httpClient.Transport.(*http2.Transport)
|
||||||
assert.True(ok)
|
assert.True(ok)
|
||||||
assert.Equal(100, transport.MaxIdleConns)
|
assert.True(transport.AllowHTTP) // Required for h2c
|
||||||
assert.Equal(50, transport.MaxConnsPerHost)
|
assert.Nil(transport.TLSClientConfig) // No TLS for http://
|
||||||
assert.Equal(10, transport.MaxIdleConnsPerHost)
|
assert.Equal(30*time.Second, transport.ReadIdleTimeout)
|
||||||
assert.Equal(30*time.Second, transport.IdleConnTimeout)
|
assert.Equal(10*time.Second, transport.PingTimeout)
|
||||||
assert.Equal(10*time.Second, transport.ResponseHeaderTimeout)
|
assert.Equal(10*time.Second, transport.WriteByteTimeout)
|
||||||
assert.False(transport.DisableKeepAlives)
|
|
||||||
assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors
|
assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors
|
||||||
assert.Equal(4096, transport.WriteBufferSize)
|
|
||||||
assert.Equal(4096, transport.ReadBufferSize)
|
|
||||||
})
|
})
|
||||||
|
|
||||||
suite.T().Run("should create HTTP/2 client for https endpoints", func(t *testing.T) {
|
suite.T().Run("should create HTTP/2 client for https endpoints", func(t *testing.T) {
|
||||||
@@ -92,23 +91,20 @@ func (suite *Tests) TestBaseClient_createHttpClient() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (suite *Tests) TestBaseClient_createHttpClient_transportSettings() {
|
func (suite *Tests) TestBaseClient_createHttpClient_transportSettings() {
|
||||||
suite.T().Run("should configure HTTP transport correctly", func(t *testing.T) {
|
suite.T().Run("should configure HTTP/2 transport correctly for http://", func(t *testing.T) {
|
||||||
client := NewConnection()
|
client := NewConnection()
|
||||||
client.SetEndpoint("http://example.com/graphql")
|
client.SetEndpoint("http://example.com/graphql")
|
||||||
|
|
||||||
httpClient := client.createHttpClient()
|
httpClient := client.createHttpClient()
|
||||||
transport := httpClient.Transport.(*http.Transport)
|
transport := httpClient.Transport.(*http2.Transport)
|
||||||
|
|
||||||
// Verify all transport settings
|
// Verify all HTTP/2 transport settings for h2c
|
||||||
assert.Equal(100, transport.MaxIdleConns)
|
assert.True(transport.AllowHTTP)
|
||||||
assert.Equal(50, transport.MaxConnsPerHost)
|
assert.Nil(transport.TLSClientConfig) // No TLS for http://
|
||||||
assert.Equal(10, transport.MaxIdleConnsPerHost)
|
assert.Equal(30*time.Second, transport.ReadIdleTimeout)
|
||||||
assert.Equal(30*time.Second, transport.IdleConnTimeout)
|
assert.Equal(10*time.Second, transport.PingTimeout)
|
||||||
assert.Equal(10*time.Second, transport.ResponseHeaderTimeout)
|
assert.Equal(10*time.Second, transport.WriteByteTimeout)
|
||||||
assert.False(transport.DisableKeepAlives)
|
|
||||||
assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors
|
assert.True(transport.DisableCompression) // Request compression disabled to prevent "trailing garbage" errors
|
||||||
assert.Equal(4096, transport.WriteBufferSize)
|
|
||||||
assert.Equal(4096, transport.ReadBufferSize)
|
|
||||||
})
|
})
|
||||||
|
|
||||||
suite.T().Run("should configure HTTP/2 transport correctly", func(t *testing.T) {
|
suite.T().Run("should configure HTTP/2 transport correctly", func(t *testing.T) {
|
||||||
@@ -129,7 +125,7 @@ func (suite *Tests) TestBaseClient_createHttpClient_transportSettings() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (suite *Tests) TestBaseClient_createHttpClient_logging() {
|
func (suite *Tests) TestBaseClient_createHttpClient_logging() {
|
||||||
suite.T().Run("should log HTTP/1.1 usage", func(t *testing.T) {
|
suite.T().Run("should log HTTP/2 h2c usage", func(t *testing.T) {
|
||||||
client := NewConnection()
|
client := NewConnection()
|
||||||
client.SetEndpoint("http://example.com/graphql")
|
client.SetEndpoint("http://example.com/graphql")
|
||||||
|
|
||||||
|
|||||||
+12
-10
@@ -11,6 +11,7 @@ import (
|
|||||||
|
|
||||||
assertions "github.com/stretchr/testify/assert"
|
assertions "github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/suite"
|
"github.com/stretchr/testify/suite"
|
||||||
|
"golang.org/x/net/http2"
|
||||||
)
|
)
|
||||||
|
|
||||||
type RequestCompressionTestSuite struct {
|
type RequestCompressionTestSuite struct {
|
||||||
@@ -74,17 +75,18 @@ func (suite *RequestCompressionTestSuite) TestRequestCompression() {
|
|||||||
httpClient := client.createHttpClient()
|
httpClient := client.createHttpClient()
|
||||||
assert.NotNil(httpClient, "HTTP client should be created")
|
assert.NotNil(httpClient, "HTTP client should be created")
|
||||||
|
|
||||||
// Verify transport settings
|
// Verify transport settings - now using HTTP/2 for all endpoints
|
||||||
if strings.HasPrefix(endpoint, "http://") {
|
transport, ok := httpClient.Transport.(*http2.Transport)
|
||||||
// HTTP/1.1 transport
|
assert.True(ok, "Should be http2.Transport for all endpoints (h2c for http://, TLS for https://)")
|
||||||
transport, ok := httpClient.Transport.(*http.Transport)
|
if ok {
|
||||||
assert.True(ok, "Should be http.Transport for HTTP endpoints")
|
assert.True(transport.DisableCompression, "DisableCompression should be true to prevent trailing garbage")
|
||||||
if ok {
|
if strings.HasPrefix(endpoint, "http://") {
|
||||||
assert.True(transport.DisableCompression, "DisableCompression should be true for HTTP transport")
|
assert.True(transport.AllowHTTP, "AllowHTTP should be true for h2c (HTTP/2 Cleartext)")
|
||||||
|
assert.Nil(transport.TLSClientConfig, "TLSClientConfig should be nil for http:// endpoints")
|
||||||
|
} else {
|
||||||
|
assert.True(transport.AllowHTTP, "AllowHTTP should be true")
|
||||||
|
assert.NotNil(transport.TLSClientConfig, "TLSClientConfig should be set for https:// endpoints")
|
||||||
}
|
}
|
||||||
} else {
|
|
||||||
// HTTP/2 transport - this is where the issue might be
|
|
||||||
// We need to ensure HTTP/2 transport also has compression disabled
|
|
||||||
}
|
}
|
||||||
|
|
||||||
qe := &QueryExecutor{
|
qe := &QueryExecutor{
|
||||||
|
|||||||
Reference in New Issue
Block a user