From e530575bc1a992b0046dd559ea95314085831540 Mon Sep 17 00:00:00 2001 From: 0x524a Date: Tue, 2 Dec 2025 01:38:50 -0500 Subject: [PATCH] refactor: improve error handling and code clarity in client methods - Enhanced error messages in the client methods to provide more context on failures. - Updated test cases to correct terminology and ensure accurate error expectations. - Refactored function signatures in media service methods for better readability and consistency. --- client.go | 15 +++++++++++---- client_test.go | 2 +- device_security.go | 4 ++++ media.go | 46 +++++++++++++++++++++++++++++++++++++--------- server/ptz_test.go | 6 ++++++ 5 files changed, 59 insertions(+), 14 deletions(-) diff --git a/client.go b/client.go index 8452fa0..c4cd57d 100644 --- a/client.go +++ b/client.go @@ -127,7 +127,7 @@ func normalizeEndpoint(endpoint string) (string, error) { // Parse as full URL parsedURL, err := url.Parse(endpoint) if err != nil { - return "", err + return "", fmt.Errorf("failed to parse endpoint URL: %w", err) } if parsedURL.Host == "" { return "", fmt.Errorf("URL missing host") @@ -284,9 +284,11 @@ func (c *Client) downloadWithBasicAuth(ctx context.Context, downloadURL string) if err != nil { return nil, fmt.Errorf("download request failed: %w", err) } + //nolint:errcheck // Close error in defer is intentionally ignored defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { + //nolint:errcheck // Error response body preview - ignore read errors bodyPreview, _ := io.ReadAll(resp.Body) bodyStr := string(bodyPreview) if len(bodyStr) > 200 { @@ -360,9 +362,11 @@ func (c *Client) downloadWithDigestAuth(ctx context.Context, downloadURL string) if err != nil { return nil, fmt.Errorf("digest auth request failed: %w", err) } + //nolint:errcheck // Close error in defer is intentionally ignored defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { + //nolint:errcheck // Error response body preview - ignore read errors bodyPreview, _ := io.ReadAll(resp.Body) bodyStr := string(bodyPreview) if len(bodyStr) > 200 { @@ -409,7 +413,7 @@ func (d *digestAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro // First request without auth to get the challenge resp, err := d.transport.RoundTrip(req) if err != nil { - return resp, err + return resp, fmt.Errorf("transport round trip failed: %w", err) } // If we get 401, handle digest auth challenge @@ -426,11 +430,14 @@ func (d *digestAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro // Retry with auth resp, err = d.transport.RoundTrip(newReq) - return resp, err + if err != nil { + return resp, fmt.Errorf("transport round trip with auth failed: %w", err) + } + return resp, nil } } - return resp, err + return resp, nil } // createDigestAuthHeader creates a digest auth header from the challenge diff --git a/client_test.go b/client_test.go index 6cb5555..f3c8a3e 100644 --- a/client_test.go +++ b/client_test.go @@ -1293,7 +1293,7 @@ func TestDownloadFileContextCancellation(t *testing.T) { _, err = client.DownloadFile(ctx, server.URL) if err == nil { - t.Error("DownloadFile() expected error for cancelled context") + t.Error("DownloadFile() expected error for canceled context") } if !strings.Contains(err.Error(), "context deadline exceeded") && !strings.Contains(err.Error(), "context canceled") { t.Errorf("Expected context error, got: %v", err) diff --git a/device_security.go b/device_security.go index 8ca0059..d702c07 100644 --- a/device_security.go +++ b/device_security.go @@ -143,6 +143,8 @@ func (c *Client) GetIPAddressFilter(ctx context.Context) (*IPAddressFilter, erro } // SetIPAddressFilter sets the IP address filter settings on a device +// +//nolint:dupl // Similar structure to AddIPAddressFilter but different operation func (c *Client) SetIPAddressFilter(ctx context.Context, filter *IPAddressFilter) error { type SetIPAddressFilter struct { XMLName xml.Name `xml:"tds:SetIPAddressFilter"` @@ -196,6 +198,8 @@ func (c *Client) SetIPAddressFilter(ctx context.Context, filter *IPAddressFilter } // AddIPAddressFilter adds an IP filter address to a device +// +//nolint:dupl // Similar structure to SetIPAddressFilter but different operation func (c *Client) AddIPAddressFilter(ctx context.Context, filter *IPAddressFilter) error { type AddIPAddressFilter struct { XMLName xml.Name `xml:"tds:AddIPAddressFilter"` diff --git a/media.go b/media.go index a7e171a..76b25f9 100644 --- a/media.go +++ b/media.go @@ -2749,7 +2749,10 @@ func (c *Client) GetAudioSourceConfiguration(ctx context.Context, configurationT } // GetVideoSourceConfigurationOptions retrieves available options for video source configuration -func (c *Client) GetVideoSourceConfigurationOptions(ctx context.Context, configurationToken, profileToken string) (*VideoSourceConfigurationOptions, error) { +func (c *Client) GetVideoSourceConfigurationOptions( + ctx context.Context, + configurationToken, profileToken string, +) (*VideoSourceConfigurationOptions, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -2809,7 +2812,10 @@ func (c *Client) GetVideoSourceConfigurationOptions(ctx context.Context, configu } // GetAudioSourceConfigurationOptions retrieves available options for audio source configuration -func (c *Client) GetAudioSourceConfigurationOptions(ctx context.Context, configurationToken, profileToken string) (*AudioSourceConfigurationOptions, error) { +func (c *Client) GetAudioSourceConfigurationOptions( + ctx context.Context, + configurationToken, profileToken string, +) (*AudioSourceConfigurationOptions, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -2854,7 +2860,11 @@ func (c *Client) GetAudioSourceConfigurationOptions(ctx context.Context, configu } // SetVideoSourceConfiguration sets video source configuration -func (c *Client) SetVideoSourceConfiguration(ctx context.Context, config *VideoSourceConfiguration, forcePersistence bool) error { +func (c *Client) SetVideoSourceConfiguration( + ctx context.Context, + config *VideoSourceConfiguration, + forcePersistence bool, +) error { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -2956,7 +2966,10 @@ func (c *Client) SetAudioSourceConfiguration(ctx context.Context, config *AudioS } // GetCompatibleVideoEncoderConfigurations retrieves compatible video encoder configurations for a profile -func (c *Client) GetCompatibleVideoEncoderConfigurations(ctx context.Context, profileToken string) ([]*VideoEncoderConfiguration, error) { +func (c *Client) GetCompatibleVideoEncoderConfigurations( + ctx context.Context, + profileToken string, +) ([]*VideoEncoderConfiguration, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -3034,7 +3047,10 @@ func (c *Client) GetCompatibleVideoEncoderConfigurations(ctx context.Context, pr } // GetCompatibleVideoSourceConfigurations retrieves compatible video source configurations for a profile -func (c *Client) GetCompatibleVideoSourceConfigurations(ctx context.Context, profileToken string) ([]*VideoSourceConfiguration, error) { +func (c *Client) GetCompatibleVideoSourceConfigurations( + ctx context.Context, + profileToken string, +) ([]*VideoSourceConfiguration, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -3099,7 +3115,10 @@ func (c *Client) GetCompatibleVideoSourceConfigurations(ctx context.Context, pro } // GetCompatibleAudioEncoderConfigurations retrieves compatible audio encoder configurations for a profile -func (c *Client) GetCompatibleAudioEncoderConfigurations(ctx context.Context, profileToken string) ([]*AudioEncoderConfiguration, error) { +func (c *Client) GetCompatibleAudioEncoderConfigurations( + ctx context.Context, + profileToken string, +) ([]*AudioEncoderConfiguration, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -3543,7 +3562,10 @@ func (c *Client) GetAudioDecoderConfigurations(ctx context.Context) ([]*AudioDec } // GetAudioDecoderConfiguration retrieves a specific audio decoder configuration -func (c *Client) GetAudioDecoderConfiguration(ctx context.Context, configurationToken string) (*AudioDecoderConfiguration, error) { +func (c *Client) GetAudioDecoderConfiguration( + ctx context.Context, + configurationToken string, +) (*AudioDecoderConfiguration, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -3671,7 +3693,10 @@ func (c *Client) GetVideoAnalyticsConfigurations(ctx context.Context) ([]*VideoA } // GetVideoAnalyticsConfiguration retrieves a specific video analytics configuration -func (c *Client) GetVideoAnalyticsConfiguration(ctx context.Context, configurationToken string) (*VideoAnalyticsConfiguration, error) { +func (c *Client) GetVideoAnalyticsConfiguration( + ctx context.Context, + configurationToken string, +) (*VideoAnalyticsConfiguration, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint @@ -3801,7 +3826,10 @@ func (c *Client) SetVideoAnalyticsConfiguration(ctx context.Context, config *Vid } // GetVideoAnalyticsConfigurationOptions retrieves available options for video analytics configuration -func (c *Client) GetVideoAnalyticsConfigurationOptions(ctx context.Context, configurationToken, profileToken string) (*VideoAnalyticsConfigurationOptions, error) { +func (c *Client) GetVideoAnalyticsConfigurationOptions( + ctx context.Context, + configurationToken, profileToken string, +) (*VideoAnalyticsConfigurationOptions, error) { endpoint := c.mediaEndpoint if endpoint == "" { endpoint = c.endpoint diff --git a/server/ptz_test.go b/server/ptz_test.go index ce3d640..d21304e 100644 --- a/server/ptz_test.go +++ b/server/ptz_test.go @@ -110,6 +110,8 @@ func _DisabledTestHandleGetStatus(t *testing.T) { } // TestHandleAbsoluteMove - DISABLED due to SOAP namespace requirements +// +//nolint:dupl // Disabled test functions have similar structure func _DisabledTestHandleAbsoluteMove(t *testing.T) { config := createTestConfig() server, _ := New(config) @@ -150,6 +152,8 @@ func _DisabledTestHandleAbsoluteMove(t *testing.T) { } // TestHandleRelativeMove - DISABLED due to SOAP namespace requirements +// +//nolint:dupl // Disabled test functions have similar structure func _DisabledTestHandleRelativeMove(t *testing.T) { config := createTestConfig() server, _ := New(config) @@ -190,6 +194,8 @@ func _DisabledTestHandleRelativeMove(t *testing.T) { } // TestHandleContinuousMove - DISABLED due to SOAP namespace requirements +// +//nolint:dupl // Disabled test functions have similar structure func _DisabledTestHandleContinuousMove(t *testing.T) { config := createTestConfig() server, _ := New(config)