Enhance error handling and metrics tracking in SteamCache
- Introduced a new error handling system with custom error types for better context and clarity in error reporting. - Implemented URL validation to prevent invalid requests and enhance security. - Updated cache key generation functions to return errors, improving robustness in handling invalid inputs. - Added comprehensive metrics tracking for requests, cache hits, misses, and performance metrics, allowing for better monitoring and analysis of the caching system. - Enhanced logging to include detailed metrics and error information for improved debugging and operational insights.
This commit is contained in:
@@ -3,6 +3,8 @@ package steamcache
|
||||
|
||||
import (
|
||||
"io"
|
||||
"s1d3sw1ped/steamcache2/steamcache/errors"
|
||||
"s1d3sw1ped/steamcache2/vfs/vfserror"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -164,10 +166,13 @@ func TestURLHashing(t *testing.T) {
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.desc, func(t *testing.T) {
|
||||
result := generateServiceCacheKey(tc.input, "steam")
|
||||
result, err := generateServiceCacheKey(tc.input, "steam")
|
||||
|
||||
if tc.shouldCache {
|
||||
// Should return a cache key with "steam/" prefix
|
||||
if err != nil {
|
||||
t.Errorf("generateServiceCacheKey(%s, \"steam\") returned error: %v", tc.input, err)
|
||||
}
|
||||
if !strings.HasPrefix(result, "steam/") {
|
||||
t.Errorf("generateServiceCacheKey(%s, \"steam\") = %s, expected steam/ prefix", tc.input, result)
|
||||
}
|
||||
@@ -176,9 +181,9 @@ func TestURLHashing(t *testing.T) {
|
||||
t.Errorf("generateServiceCacheKey(%s, \"steam\") length = %d, expected 70", tc.input, len(result))
|
||||
}
|
||||
} else {
|
||||
// Should return empty string for non-Steam URLs
|
||||
if result != "" {
|
||||
t.Errorf("generateServiceCacheKey(%s, \"steam\") = %s, expected empty string", tc.input, result)
|
||||
// Should return error for invalid URLs
|
||||
if err == nil {
|
||||
t.Errorf("generateServiceCacheKey(%s, \"steam\") should have returned error", tc.input)
|
||||
}
|
||||
}
|
||||
})
|
||||
@@ -322,8 +327,14 @@ func TestServiceManagerExpandability(t *testing.T) {
|
||||
}
|
||||
|
||||
// Test cache key generation for different services
|
||||
steamKey := generateServiceCacheKey("/depot/123/chunk/abc", "steam")
|
||||
epicKey := generateServiceCacheKey("/epic/123/chunk/abc", "epic")
|
||||
steamKey, err := generateServiceCacheKey("/depot/123/chunk/abc", "steam")
|
||||
if err != nil {
|
||||
t.Errorf("Failed to generate Steam cache key: %v", err)
|
||||
}
|
||||
epicKey, err := generateServiceCacheKey("/epic/123/chunk/abc", "epic")
|
||||
if err != nil {
|
||||
t.Errorf("Failed to generate Epic cache key: %v", err)
|
||||
}
|
||||
|
||||
if !strings.HasPrefix(steamKey, "steam/") {
|
||||
t.Errorf("Steam cache key should start with 'steam/', got: %s", steamKey)
|
||||
@@ -367,4 +378,139 @@ func TestSteamKeySharding(t *testing.T) {
|
||||
// and be readable, whereas without sharding it might not work correctly
|
||||
}
|
||||
|
||||
// TestURLValidation tests the URL validation function
|
||||
func TestURLValidation(t *testing.T) {
|
||||
testCases := []struct {
|
||||
urlPath string
|
||||
shouldPass bool
|
||||
description string
|
||||
}{
|
||||
{
|
||||
urlPath: "/depot/123/chunk/abc",
|
||||
shouldPass: true,
|
||||
description: "valid Steam URL",
|
||||
},
|
||||
{
|
||||
urlPath: "/appinfo/456",
|
||||
shouldPass: true,
|
||||
description: "valid app info URL",
|
||||
},
|
||||
{
|
||||
urlPath: "",
|
||||
shouldPass: false,
|
||||
description: "empty URL",
|
||||
},
|
||||
{
|
||||
urlPath: "/depot/../etc/passwd",
|
||||
shouldPass: false,
|
||||
description: "directory traversal attempt",
|
||||
},
|
||||
{
|
||||
urlPath: "/depot//123/chunk/abc",
|
||||
shouldPass: false,
|
||||
description: "double slash",
|
||||
},
|
||||
{
|
||||
urlPath: "/depot/123/chunk/abc<script>",
|
||||
shouldPass: false,
|
||||
description: "suspicious characters",
|
||||
},
|
||||
{
|
||||
urlPath: strings.Repeat("/depot/123/chunk/abc", 200), // This will be much longer than 2048 chars
|
||||
shouldPass: false,
|
||||
description: "URL too long",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.description, func(t *testing.T) {
|
||||
err := validateURLPath(tc.urlPath)
|
||||
if tc.shouldPass && err != nil {
|
||||
t.Errorf("validateURLPath(%q) should pass but got error: %v", tc.urlPath, err)
|
||||
}
|
||||
if !tc.shouldPass && err == nil {
|
||||
t.Errorf("validateURLPath(%q) should fail but passed", tc.urlPath)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestErrorTypes tests the custom error types
|
||||
func TestErrorTypes(t *testing.T) {
|
||||
// Test VFS error
|
||||
vfsErr := vfserror.NewVFSError("test", "key1", vfserror.ErrNotFound)
|
||||
if vfsErr.Error() == "" {
|
||||
t.Error("VFS error should have a message")
|
||||
}
|
||||
if vfsErr.Unwrap() != vfserror.ErrNotFound {
|
||||
t.Error("VFS error should unwrap to the underlying error")
|
||||
}
|
||||
|
||||
// Test SteamCache error
|
||||
scErr := errors.NewSteamCacheError("test", "/test/url", "127.0.0.1", errors.ErrInvalidURL)
|
||||
if scErr.Error() == "" {
|
||||
t.Error("SteamCache error should have a message")
|
||||
}
|
||||
if scErr.Unwrap() != errors.ErrInvalidURL {
|
||||
t.Error("SteamCache error should unwrap to the underlying error")
|
||||
}
|
||||
|
||||
// Test retryable error detection
|
||||
if !errors.IsRetryableError(errors.ErrUpstreamUnavailable) {
|
||||
t.Error("Upstream unavailable should be retryable")
|
||||
}
|
||||
if errors.IsRetryableError(errors.ErrInvalidURL) {
|
||||
t.Error("Invalid URL should not be retryable")
|
||||
}
|
||||
}
|
||||
|
||||
// TestMetrics tests the metrics functionality
|
||||
func TestMetrics(t *testing.T) {
|
||||
td := t.TempDir()
|
||||
sc := New("localhost:8080", "1G", "1G", td, "", "lru", "lru", 200, 5)
|
||||
|
||||
// Test initial metrics
|
||||
stats := sc.GetMetrics()
|
||||
if stats.TotalRequests != 0 {
|
||||
t.Error("Initial total requests should be 0")
|
||||
}
|
||||
if stats.CacheHits != 0 {
|
||||
t.Error("Initial cache hits should be 0")
|
||||
}
|
||||
|
||||
// Test metrics increment
|
||||
sc.metrics.IncrementTotalRequests()
|
||||
sc.metrics.IncrementCacheHits()
|
||||
sc.metrics.IncrementCacheMisses()
|
||||
sc.metrics.AddBytesServed(1024)
|
||||
sc.metrics.IncrementServiceRequests("steam")
|
||||
|
||||
stats = sc.GetMetrics()
|
||||
if stats.TotalRequests != 1 {
|
||||
t.Error("Total requests should be 1")
|
||||
}
|
||||
if stats.CacheHits != 1 {
|
||||
t.Error("Cache hits should be 1")
|
||||
}
|
||||
if stats.CacheMisses != 1 {
|
||||
t.Error("Cache misses should be 1")
|
||||
}
|
||||
if stats.TotalBytesServed != 1024 {
|
||||
t.Error("Total bytes served should be 1024")
|
||||
}
|
||||
if stats.ServiceRequests["steam"] != 1 {
|
||||
t.Error("Steam service requests should be 1")
|
||||
}
|
||||
|
||||
// Test metrics reset
|
||||
sc.ResetMetrics()
|
||||
stats = sc.GetMetrics()
|
||||
if stats.TotalRequests != 0 {
|
||||
t.Error("After reset, total requests should be 0")
|
||||
}
|
||||
if stats.CacheHits != 0 {
|
||||
t.Error("After reset, cache hits should be 0")
|
||||
}
|
||||
}
|
||||
|
||||
// Removed old TestKeyGeneration - replaced with TestURLHashing that uses SHA256
|
||||
|
||||
Reference in New Issue
Block a user