From 9942f6073a0864e30b0393e4a074dd8c062e9a56 Mon Sep 17 00:00:00 2001 From: Matteo Valentini Date: Fri, 13 Mar 2026 14:47:50 +0100 Subject: [PATCH 1/5] fix: handle grape/sraps permission errors correctly Parse APIError responses to map permission errors (taken_mac_or_forbidden, User lacks permission to endpoint) to the semantic error code device_owned_by_other_user. This improves error clarity for clients when a MAC address is registered in another account. Changes: - Add pattern matching in ParseProviderError for grape/sraps permission errors - Implement error parsing in both GrapeDevice and SrapsDevice Register methods - Call ParseProviderError on APIError to get semantic error codes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- providers/grape.go | 15 ++++++++++++--- providers/sraps.go | 15 ++++++++++++--- utils/utils.go | 4 +++- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/providers/grape.go b/providers/grape.go index a706f05..31fac87 100644 --- a/providers/grape.go +++ b/providers/grape.go @@ -34,6 +34,7 @@ import ( "github.com/nethesis/falconieri/configuration" "github.com/nethesis/falconieri/libs/grape" "github.com/nethesis/falconieri/models" + "github.com/nethesis/falconieri/utils" ) var ( @@ -68,10 +69,18 @@ func (d GrapeDevice) Register() error { // Check if this is an API error (4xx/5xx from server) var apiErr grape.APIError if errors.As(err, &apiErr) { - return models.ProviderError{ - Message: "provider_remote_call_failed", - WrappedError: apiErr, + // Parse the API error to map it to semantic error codes + parsedErr := utils.ParseProviderError(apiErr.Error()) + if providerErr, ok := parsedErr.(models.ProviderError); ok { + // Already a semantic error from ParseProviderError + return providerErr + } + if _, ok := parsedErr.(*models.ProviderError); ok { + // Already a semantic error from ParseProviderError + return parsedErr } + // Return semantic error (e.g., "device_owned_by_other_user") + return parsedErr } // Check if this is a transport-level error (DNS, connection, timeout) diff --git a/providers/sraps.go b/providers/sraps.go index d9f204c..23f66bc 100644 --- a/providers/sraps.go +++ b/providers/sraps.go @@ -35,6 +35,7 @@ import ( "github.com/nethesis/falconieri/configuration" "github.com/nethesis/falconieri/libs/grape" "github.com/nethesis/falconieri/models" + "github.com/nethesis/falconieri/utils" ) var ( @@ -69,10 +70,18 @@ func (d SrapsDevice) Register() error { if err != nil { var apiErr grape.APIError if errors.As(err, &apiErr) { - return models.ProviderError{ - Message: "provider_remote_call_failed", - WrappedError: apiErr, + // Parse the API error to map it to semantic error codes + parsedErr := utils.ParseProviderError(apiErr.Error()) + if providerErr, ok := parsedErr.(models.ProviderError); ok { + // Already a semantic error from ParseProviderError + return providerErr + } + if _, ok := parsedErr.(*models.ProviderError); ok { + // Already a semantic error from ParseProviderError + return parsedErr } + // Return semantic error (e.g., "device_owned_by_other_user") + return parsedErr } var urlErr *url.Error diff --git a/utils/utils.go b/utils/utils.go index 98d5da9..1ba2f8d 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -79,7 +79,9 @@ func ParseProviderError(message string) error { message == "800004", //ymcs message == "800003", //ymcs: Resource already exists strings.Contains(strings.ToLower(message), "device already managed"), - strings.Contains(strings.ToLower(message), "resource already exists"): + strings.Contains(strings.ToLower(message), "resource already exists"), + strings.Contains(strings.ToLower(message), "taken_mac_or_forbidden"), //grape/sraps + strings.Contains(strings.ToLower(message), "permission") && strings.Contains(strings.ToLower(message), "endpoint"): //grape/sraps: "User lacks permission to endpoint" return errors.New("device_owned_by_other_user") From c9d4b19c7334c023974c35772340f99aafdf7773 Mon Sep 17 00:00:00 2001 From: Matteo Valentini Date: Fri, 13 Mar 2026 15:05:44 +0100 Subject: [PATCH 2/5] fix: rename 'GRAPE API error' prefix to generic 'API error' The error message was hardcoded to 'GRAPE API error (HTTP X): ...' even when the library is used by the SRAPS provider, making error messages misleading in that context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- libs/grape/errors.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/grape/errors.go b/libs/grape/errors.go index 66969ef..455805b 100644 --- a/libs/grape/errors.go +++ b/libs/grape/errors.go @@ -38,9 +38,9 @@ type APIError struct { // Error implements the error interface func (e APIError) Error() string { if e.Message != "" { - return fmt.Sprintf("GRAPE API error (HTTP %d): %s", e.StatusCode, e.Message) + return fmt.Sprintf("API error (HTTP %d): %s", e.StatusCode, e.Message) } - return fmt.Sprintf("GRAPE API error (HTTP %d): %s", e.StatusCode, e.Status) + return fmt.Sprintf("API error (HTTP %d): %s", e.StatusCode, e.Status) } // parseErrorResponse attempts to extract error information from the response body From da799fa51c18bc6d27dfa9bf0b4e9cbf5514a78f Mon Sep 17 00:00:00 2001 From: Matteo Valentini Date: Fri, 13 Mar 2026 15:05:52 +0100 Subject: [PATCH 3/5] fix: omit warranty_exp_warning_period from PUT request body The field was serialized as null, which on a full-update PUT explicitly resets the server-side default (3 months) for all registered devices. Adding omitempty ensures the field is omitted when no value is set, leaving the server default intact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- libs/grape/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/grape/types.go b/libs/grape/types.go index bf9f14e..3ad92a4 100644 --- a/libs/grape/types.go +++ b/libs/grape/types.go @@ -46,6 +46,6 @@ type CompanyResponse struct { type DeviceData struct { MAC string `json:"mac"` AutoprovisioningEnabled bool `json:"autoprovisioning_enabled"` - WarrantyExpWarningPeriod *int `json:"warranty_exp_warning_period"` + WarrantyExpWarningPeriod *int `json:"warranty_exp_warning_period,omitempty"` SettingsManager map[string]map[string]interface{} `json:"settings_manager"` } From bf90d3a85e3274c87cf83cb62b3d0decec35756a Mon Sep 17 00:00:00 2001 From: Matteo Valentini Date: Fri, 13 Mar 2026 15:06:01 +0100 Subject: [PATCH 4/5] fix: paginate settings list when looking up setting UUID The GRAPE/SRAPS settings API paginates with a default page size of 20 and a maximum of 1000. With 1179 global settings, fetching only the first page could silently miss the provisioning setting UUID, causing all registrations to fail on accounts where the setting appears beyond page 1. Changes: - Add makeHawkRequestFull() that returns both body and response headers alongside the existing makeHawkRequest() wrapper. - Rewrite getProvisioningServerID() to follow RFC 5988 Link headers (rel="next") until the setting is found or pages are exhausted. - Use page_size=1000 on the initial request to minimise round-trips. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- libs/grape/auth.go | 25 ++++++++++------ libs/grape/devices.go | 69 +++++++++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 31 deletions(-) diff --git a/libs/grape/auth.go b/libs/grape/auth.go index 50fc69a..0c9f92d 100644 --- a/libs/grape/auth.go +++ b/libs/grape/auth.go @@ -130,8 +130,9 @@ func createHawkAuthHeader(creds *HawkCredentials, method, rawURL string, payload return authHeader, nil } -// makeHawkRequest performs an HTTP request with Hawk authentication -func (c *Client) makeHawkRequest(method, url string, body []byte) ([]byte, error) { +// makeHawkRequestFull performs an HTTP request with Hawk authentication and +// returns both the response body and headers (needed for Link-based pagination). +func (c *Client) makeHawkRequestFull(method, url string, body []byte) ([]byte, http.Header, error) { var req *http.Request var err error var contentType string @@ -139,14 +140,14 @@ func (c *Client) makeHawkRequest(method, url string, body []byte) ([]byte, error if body != nil { req, err = http.NewRequest(method, url, bytes.NewReader(body)) if err != nil { - return nil, err + return nil, nil, err } contentType = "application/json" req.Header.Set("Content-Type", contentType) } else { req, err = http.NewRequest(method, url, nil) if err != nil { - return nil, err + return nil, nil, err } contentType = "" } @@ -162,7 +163,7 @@ func (c *Client) makeHawkRequest(method, url string, body []byte) ([]byte, error // Add Hawk authentication header authHeader, err := createHawkAuthHeader(creds, method, url, body, contentType) if err != nil { - return nil, fmt.Errorf("failed to create Hawk auth header: %v", err) + return nil, nil, fmt.Errorf("failed to create Hawk auth header: %v", err) } req.Header.Set("Authorization", authHeader) @@ -177,13 +178,13 @@ func (c *Client) makeHawkRequest(method, url string, body []byte) ([]byte, error resp, err := c.HTTPClient.Do(req) if err != nil { - return nil, err + return nil, nil, err } defer resp.Body.Close() bodyBytes, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, nil, fmt.Errorf("failed to read response body: %w", err) } // Store response for debugging @@ -195,8 +196,14 @@ func (c *Client) makeHawkRequest(method, url string, body []byte) ([]byte, error } if resp.StatusCode >= 400 { - return nil, parseErrorResponse(resp.StatusCode, resp.Status, bodyBytes) + return nil, nil, parseErrorResponse(resp.StatusCode, resp.Status, bodyBytes) } - return bodyBytes, nil + return bodyBytes, resp.Header, nil +} + +// makeHawkRequest performs an HTTP request with Hawk authentication +func (c *Client) makeHawkRequest(method, url string, body []byte) ([]byte, error) { + bodyBytes, _, err := c.makeHawkRequestFull(method, url, body) + return bodyBytes, err } diff --git a/libs/grape/devices.go b/libs/grape/devices.go index 7a09b95..eae5b57 100644 --- a/libs/grape/devices.go +++ b/libs/grape/devices.go @@ -25,6 +25,7 @@ package grape import ( "encoding/json" "fmt" + "strings" ) // getProvisioningServerID retrieves and caches the provisioning setting UUID. @@ -33,39 +34,63 @@ import ( // Uses sync.Once to prevent duplicate API calls under concurrent load. func (c *Client) getProvisioningServerID() (string, error) { c.provisioningServerIDOnce.Do(func() { - // Get settings to find the provisioning setting UUID - settingsURL := c.BaseURL + "settings/" - settings, err := c.makeHawkRequest("GET", settingsURL, nil) - if err != nil { - c.provisioningServerIDErr = fmt.Errorf("failed to get settings: %w", err) - return - } + // Fetch all settings pages to find the provisioning setting UUID. + // The API paginates with default 20 items; use page_size=1000 to + // minimise round-trips. Pagination continues while the response + // contains a Link header with rel="next". + nextURL := c.BaseURL + "settings/?page_size=1000" + for nextURL != "" { + pageBytes, headers, err := c.makeHawkRequestFull("GET", nextURL, nil) + if err != nil { + c.provisioningServerIDErr = fmt.Errorf("failed to get settings: %w", err) + return + } - var settingsList []Setting - if err := json.Unmarshal(settings, &settingsList); err != nil { - c.provisioningServerIDErr = fmt.Errorf("failed to parse settings response: %w", err) - return - } + var page []Setting + if err := json.Unmarshal(pageBytes, &page); err != nil { + c.provisioningServerIDErr = fmt.Errorf("failed to parse settings response: %w", err) + return + } - var settingProvisioningServerUUID string - for _, setting := range settingsList { - if setting.ParamName == c.ProvisioningSettingName { - settingProvisioningServerUUID = setting.UUID - break + for _, setting := range page { + if setting.ParamName == c.ProvisioningSettingName { + c.provisioningServerID = setting.UUID + return + } } - } - if settingProvisioningServerUUID == "" { - c.provisioningServerIDErr = fmt.Errorf("%s setting not found in API response", c.ProvisioningSettingName) - return + // Follow Link header for next page if present + nextURL = parseLinkNext(headers.Get("Link")) } - c.provisioningServerID = settingProvisioningServerUUID + c.provisioningServerIDErr = fmt.Errorf("%s setting not found in API response", c.ProvisioningSettingName) }) return c.provisioningServerID, c.provisioningServerIDErr } +// parseLinkNext extracts the URL for rel="next" from an RFC 5988 Link header. +// Returns an empty string if there is no next page. +func parseLinkNext(linkHeader string) string { + // Format: ; rel="next", ; rel="prev" + for _, part := range strings.Split(linkHeader, ",") { + part = strings.TrimSpace(part) + segments := strings.Split(part, ";") + if len(segments) < 2 { + continue + } + for _, seg := range segments[1:] { + if strings.TrimSpace(seg) == `rel="next"` { + url := strings.TrimSpace(segments[0]) + url = strings.TrimPrefix(url, "<") + url = strings.TrimSuffix(url, ">") + return url + } + } + } + return "" +} + // getEndpointsURL retrieves and caches the endpoints URL for device operations // Uses sync.Once to prevent duplicate API calls under concurrent load func (c *Client) getEndpointsURL() (string, error) { From 4ffc18dbc110ecb01a64b1383e19cb35e602d897 Mon Sep 17 00:00:00 2001 From: Matteo Valentini Date: Fri, 13 Mar 2026 16:00:55 +0100 Subject: [PATCH 5/5] fixup! fix: handle grape/sraps permission errors correctly Remove dead type-assertion blocks after ParseProviderError call. All three branches returned parsedErr unchanged; the *models.ProviderError pointer assertion can never match since ParseProviderError never returns a pointer. Simplify to a direct return. Review suggestions: - https://github.com/nethesis/falconieri/pull/51#discussion_r2931740180 (grape.go) - https://github.com/nethesis/falconieri/pull/51#discussion_r2931740243 (sraps.go) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- providers/grape.go | 13 +------------ providers/sraps.go | 13 +------------ 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/providers/grape.go b/providers/grape.go index 31fac87..97399e3 100644 --- a/providers/grape.go +++ b/providers/grape.go @@ -69,18 +69,7 @@ func (d GrapeDevice) Register() error { // Check if this is an API error (4xx/5xx from server) var apiErr grape.APIError if errors.As(err, &apiErr) { - // Parse the API error to map it to semantic error codes - parsedErr := utils.ParseProviderError(apiErr.Error()) - if providerErr, ok := parsedErr.(models.ProviderError); ok { - // Already a semantic error from ParseProviderError - return providerErr - } - if _, ok := parsedErr.(*models.ProviderError); ok { - // Already a semantic error from ParseProviderError - return parsedErr - } - // Return semantic error (e.g., "device_owned_by_other_user") - return parsedErr + return utils.ParseProviderError(apiErr.Error()) } // Check if this is a transport-level error (DNS, connection, timeout) diff --git a/providers/sraps.go b/providers/sraps.go index 23f66bc..3a5f113 100644 --- a/providers/sraps.go +++ b/providers/sraps.go @@ -70,18 +70,7 @@ func (d SrapsDevice) Register() error { if err != nil { var apiErr grape.APIError if errors.As(err, &apiErr) { - // Parse the API error to map it to semantic error codes - parsedErr := utils.ParseProviderError(apiErr.Error()) - if providerErr, ok := parsedErr.(models.ProviderError); ok { - // Already a semantic error from ParseProviderError - return providerErr - } - if _, ok := parsedErr.(*models.ProviderError); ok { - // Already a semantic error from ParseProviderError - return parsedErr - } - // Return semantic error (e.g., "device_owned_by_other_user") - return parsedErr + return utils.ParseProviderError(apiErr.Error()) } var urlErr *url.Error