feat: add OperationFailedError with actionable hints
The Kanboard API returns only true/false for many operations without explaining why they failed. Added OperationFailedError type that includes operation details and hints about possible causes. Updated MoveTaskPosition and MoveTaskToProject to use this new error type, providing users with actionable debugging information instead of generic "failed to move task" messages. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
508c3ac6d2
commit
449cd2626c
6 changed files with 131 additions and 5 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"worktree_root": "/home/oli/Dev/kanboard-api",
|
||||
"last_export_commit": "96601980c3838ba7e38859330dfcc3d8183b35c1",
|
||||
"last_export_time": "2026-01-27T11:01:28.430072224+01:00",
|
||||
"jsonl_hash": "3136cc76a92ba36a130a3ba6a13f3bb239b7a54f6d3a345ae64815d3ec602ffb"
|
||||
"last_export_commit": "508c3ac6d2f3cd76cdbd540b724e8efb6c74bd0c",
|
||||
"last_export_time": "2026-01-27T11:12:10.369070811+01:00",
|
||||
"jsonl_hash": "a6d0e597873b206d5a2394753518ab6bcf07158f415728c695d08083c5e39d0b"
|
||||
}
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
{"id":"kanboard-1cb","title":"Investigate MoveTaskPosition generic error message","description":"## Context\n\nThis is an investigation task to determine if a problem in the `hqcli` tool (which uses this library) has its root cause here.\n\nThe `kb ticket move` command in hqcli fails with an unhelpful error:\n\n```\n❯ dist/hqcli kb ticket move 3765\n2026/01/27 11:02:43 Fehler beim Verschieben des Tickets: moveTaskPosition: failed to move task 3765\n```\n\n## Library Behavior\n\nLooking at `tasks.go:131-150`, the `MoveTaskPosition` function:\n\n```go\nfunc (c *Client) MoveTaskPosition(...) error {\n // ...\n var success bool\n if err := c.call(ctx, \"moveTaskPosition\", params, \u0026success); err != nil {\n return fmt.Errorf(\"moveTaskPosition: %w\", err)\n }\n\n if !success {\n return fmt.Errorf(\"moveTaskPosition: failed to move task %d\", taskID) // \u003c-- generic error\n }\n return nil\n}\n```\n\nWhen the API returns `false`, the error message is completely generic with no actionable information.\n\n## Investigation Needed\n\n1. **API Response Analysis**: What does Kanboard actually return when `moveTaskPosition` fails?\n - Does it include an error message in the JSON-RPC response?\n - Is the `false` result the only indication of failure?\n\n2. **Possible Causes for API returning false**:\n - Invalid column/project combination\n - Permission issues\n - Task doesn't exist\n - Position 0 might have different semantics than documented\n\n3. **Documentation Check**: [Kanboard API docs](https://docs.kanboard.org/v1/api/task_procedures/#movetaskposition)\n\n## Potential Improvements\n\nIf the API provides no additional details, consider:\n- Adding debug logging to show the full API response\n- Checking if the task/column/project exist before calling\n- Providing a more descriptive error: \"moveTaskPosition returned false - verify task exists, column belongs to project, and user has permission\"\n\n## Acceptance Criteria\n\n- [ ] Root cause identified (is it library issue or hqcli usage issue?)\n- [ ] If library issue: improve error message or error handling\n- [ ] Document findings","status":"closed","priority":2,"issue_type":"task","owner":"mail@oliverjakoubek.de","created_at":"2026-01-27T11:05:16.089428488+01:00","created_by":"Oliver Jakoubek","updated_at":"2026-01-27T11:11:58.26724838+01:00","closed_at":"2026-01-27T11:11:58.26724838+01:00","close_reason":"Closed"}
|
||||
{"id":"kanboard-7es","title":"JSON-RPC Request-ID: Zufälligen Wert statt fester 1 verwenden","description":"## Kontext\n\nIn jedem JSON-RPC Request an die Kanboard-API wird im Root-Objekt ein Feld `id` mitgeliefert. Dieses dient dazu, bei asynchroner Kommunikation Request und Response einander zuordnen zu können – die API liefert diese ID in der Antwort zurück.\n\n**Aktuell:** Die ID ist fest auf `1` gesetzt.\n\n## Anforderung\n\n1. **Wenn keine ID von außen gesetzt wird:** Die Library soll intern einen zufälligen Wert generieren\n2. **API-Dokumentation prüfen:** Welche Werte sind erlaubt? Welche Größenordnung? (vermutlich Integer)\n3. **Signatur beibehalten:** Die öffentliche API der Library-Funktionen soll unverändert bleiben\n4. **Interne Generierung:** Die Library bestimmt selbst einen zufälligen Wert\n\n## Implementierungshinweise\n\n- Prüfen: Kanboard JSON-RPC Dokumentation bezüglich erlaubter ID-Werte\n- Vermutlich: `int64` oder `int32` Range\n- Zufallsgenerator: `math/rand` mit Seed oder `crypto/rand` für bessere Verteilung\n- Ggf. bestehende `requestIDCounter` in `jsonrpc.go` (Zeile 40) anpassen oder ersetzen\n\n## Beispiel\n\n**Vorher (immer gleich):**\n```json\n{\"jsonrpc\": \"2.0\", \"method\": \"getTask\", \"id\": 1, \"params\": {...}}\n```\n\n**Nachher (zufällig):**\n```json\n{\"jsonrpc\": \"2.0\", \"method\": \"getTask\", \"id\": 847291536, \"params\": {...}}\n```\n\n## Referenz\n\n- Datei: `jsonrpc.go`\n- Zeile 17: `ID int64 \\`json:\"id\"\\``\n- Zeile 40: `requestIDCounter` (existiert bereits)","status":"closed","priority":2,"issue_type":"feature","owner":"mail@oliverjakoubek.de","created_at":"2026-01-23T17:44:51.566737509+01:00","created_by":"Oliver Jakoubek","updated_at":"2026-01-27T10:21:06.415129879+01:00","closed_at":"2026-01-27T10:21:06.415129879+01:00","close_reason":"Closed"}
|
||||
{"id":"kanboard-9wa","title":"Support custom authentication header name","description":"## Description\n\nKanboard supports using an alternative HTTP header for authentication when the server has specific configuration requirements.\n\nCurrently, authentication uses the standard `Authorization` header via Go's `SetBasicAuth()`. This needs to be configurable so users can specify a custom header name (e.g., `X-API-Auth`).\n\n## Requirements\n\n- Add an optional configuration parameter for custom auth header name\n- Default to standard `Authorization` header if not specified\n- When custom header is set, use that header name instead of `Authorization`\n- The header value format should remain the same (Basic Auth base64-encoded credentials)\n\n## Acceptance Criteria\n\n- [ ] New client configuration method (e.g., `WithAuthHeader(headerName string)`)\n- [ ] Default behavior unchanged when no custom header specified\n- [ ] Custom header name is used when configured\n- [ ] Works with both API token and basic auth\n- [ ] Tests cover default and custom header scenarios\n\n## Reference\n\nKanboard API documentation: \"You can use an alternative HTTP header for authentication if your server has a very specific configuration.\"","status":"closed","priority":2,"issue_type":"feature","owner":"mail@oliverjakoubek.de","created_at":"2026-01-23T18:08:31.507616093+01:00","created_by":"Oliver Jakoubek","updated_at":"2026-01-23T18:26:50.40804952+01:00","closed_at":"2026-01-23T18:26:50.40804952+01:00","close_reason":"Closed"}
|
||||
{"id":"kanboard-a7x","title":"Handle URLs already ending in /jsonrpc.php","description":"## Context\n\nThe `NewClient()` function in `client.go` always appends `/jsonrpc.php` to the base URL. This causes issues when users pass a URL that already includes the endpoint path.\n\n## Problem\n\nIf a user calls:\n```go\nclient := kanboard.NewClient(\"https://example.com/jsonrpc.php\")\n```\n\nThe resulting endpoint becomes `https://example.com/jsonrpc.php/jsonrpc.php`, which fails.\n\n## Solution\n\nModify `NewClient()` to detect and handle URLs that already end in `/jsonrpc.php`:\n\n```go\nfunc NewClient(baseURL string) *Client {\n // Ensure no trailing slash\n baseURL = strings.TrimSuffix(baseURL, \"/\")\n\n // Handle URLs that already include /jsonrpc.php\n endpoint := baseURL\n if !strings.HasSuffix(baseURL, \"/jsonrpc.php\") {\n endpoint = baseURL + \"/jsonrpc.php\"\n }\n\n c := \u0026Client{\n baseURL: baseURL,\n endpoint: endpoint,\n }\n // ... rest unchanged\n}\n```\n\n## Files to Modify\n\n- `client.go` - Update `NewClient()` to check for existing `/jsonrpc.php` suffix\n\n## Acceptance Criteria\n\n- [ ] `NewClient(\"https://example.com\")` → endpoint `https://example.com/jsonrpc.php`\n- [ ] `NewClient(\"https://example.com/\")` → endpoint `https://example.com/jsonrpc.php`\n- [ ] `NewClient(\"https://example.com/jsonrpc.php\")` → endpoint `https://example.com/jsonrpc.php`\n- [ ] `NewClient(\"https://example.com/kanboard/jsonrpc.php\")` → endpoint `https://example.com/kanboard/jsonrpc.php`\n- [ ] Tests written and passing","status":"closed","priority":2,"issue_type":"bug","owner":"mail@oliverjakoubek.de","created_at":"2026-01-27T10:25:51.077352962+01:00","created_by":"Oliver Jakoubek","updated_at":"2026-01-27T10:27:15.189568683+01:00","closed_at":"2026-01-27T10:27:15.189568683+01:00","close_reason":"Closed"}
|
||||
|
|
|
|||
26
errors.go
26
errors.go
|
|
@ -82,6 +82,26 @@ func (e *APIError) Error() string {
|
|||
return fmt.Sprintf("Kanboard API error (code %d): %s", e.Code, e.Message)
|
||||
}
|
||||
|
||||
// OperationFailedError represents an API operation that returned false without details.
|
||||
// The Kanboard API often returns only true/false without explaining why an operation failed.
|
||||
type OperationFailedError struct {
|
||||
Operation string
|
||||
Hints []string
|
||||
}
|
||||
|
||||
// Error implements the error interface.
|
||||
func (e *OperationFailedError) Error() string {
|
||||
msg := fmt.Sprintf("%s: operation failed", e.Operation)
|
||||
if len(e.Hints) > 0 {
|
||||
msg += " (possible causes: " + e.Hints[0]
|
||||
for _, hint := range e.Hints[1:] {
|
||||
msg += ", " + hint
|
||||
}
|
||||
msg += ")"
|
||||
}
|
||||
return msg
|
||||
}
|
||||
|
||||
// IsNotFound returns true if the error indicates a resource was not found.
|
||||
func IsNotFound(err error) bool {
|
||||
return errors.Is(err, ErrNotFound) ||
|
||||
|
|
@ -102,3 +122,9 @@ func IsAPIError(err error) bool {
|
|||
var apiErr *APIError
|
||||
return errors.As(err, &apiErr)
|
||||
}
|
||||
|
||||
// IsOperationFailed returns true if the error is an OperationFailedError.
|
||||
func IsOperationFailed(err error) bool {
|
||||
var opErr *OperationFailedError
|
||||
return errors.As(err, &opErr)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package kanboard
|
|||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
|
|
@ -217,3 +218,71 @@ func TestErrorWrapping(t *testing.T) {
|
|||
t.Errorf("unexpected error message: %s", wrappedTwice.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestOperationFailedError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err *OperationFailedError
|
||||
expectedSubstr []string
|
||||
}{
|
||||
{
|
||||
name: "with hints",
|
||||
err: &OperationFailedError{
|
||||
Operation: "moveTaskPosition(task=42, column=5, project=1)",
|
||||
Hints: []string{"task may not exist", "column may not belong to project"},
|
||||
},
|
||||
expectedSubstr: []string{
|
||||
"moveTaskPosition",
|
||||
"operation failed",
|
||||
"possible causes",
|
||||
"task may not exist",
|
||||
"column may not belong to project",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "without hints",
|
||||
err: &OperationFailedError{
|
||||
Operation: "someOperation",
|
||||
Hints: nil,
|
||||
},
|
||||
expectedSubstr: []string{"someOperation", "operation failed"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
errMsg := tt.err.Error()
|
||||
for _, substr := range tt.expectedSubstr {
|
||||
if !containsSubstr(errMsg, substr) {
|
||||
t.Errorf("error message %q should contain %q", errMsg, substr)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsOperationFailed(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
expected bool
|
||||
}{
|
||||
{"OperationFailedError", &OperationFailedError{Operation: "test"}, true},
|
||||
{"wrapped OperationFailedError", fmt.Errorf("call failed: %w", &OperationFailedError{Operation: "test"}), true},
|
||||
{"ErrUnauthorized", ErrUnauthorized, false},
|
||||
{"generic error", errors.New("some error"), false},
|
||||
{"nil", nil, false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := IsOperationFailed(tt.err); got != tt.expected {
|
||||
t.Errorf("IsOperationFailed(%v) = %v, want %v", tt.err, got, tt.expected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func containsSubstr(s, substr string) bool {
|
||||
return strings.Contains(s, substr)
|
||||
}
|
||||
|
|
|
|||
19
tasks.go
19
tasks.go
|
|
@ -143,7 +143,15 @@ func (c *Client) MoveTaskPosition(ctx context.Context, projectID, taskID, column
|
|||
}
|
||||
|
||||
if !success {
|
||||
return fmt.Errorf("moveTaskPosition: failed to move task %d", taskID)
|
||||
return &OperationFailedError{
|
||||
Operation: fmt.Sprintf("moveTaskPosition(task=%d, column=%d, project=%d)", taskID, columnID, projectID),
|
||||
Hints: []string{
|
||||
"task may not exist",
|
||||
"column may not belong to project",
|
||||
"insufficient permissions",
|
||||
"task may already be in target position",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
@ -162,7 +170,14 @@ func (c *Client) MoveTaskToProject(ctx context.Context, taskID, projectID int) e
|
|||
}
|
||||
|
||||
if !success {
|
||||
return fmt.Errorf("moveTaskToProject: failed to move task %d to project %d", taskID, projectID)
|
||||
return &OperationFailedError{
|
||||
Operation: fmt.Sprintf("moveTaskToProject(task=%d, project=%d)", taskID, projectID),
|
||||
Hints: []string{
|
||||
"task may not exist",
|
||||
"target project may not exist",
|
||||
"insufficient permissions",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import (
|
|||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
|
|
@ -639,6 +640,20 @@ func TestClient_MoveTaskPosition_Failure(t *testing.T) {
|
|||
if err == nil {
|
||||
t.Fatal("expected error for failed move")
|
||||
}
|
||||
|
||||
// Verify it's an OperationFailedError with helpful hints
|
||||
if !IsOperationFailed(err) {
|
||||
t.Errorf("expected OperationFailedError, got %T", err)
|
||||
}
|
||||
|
||||
// Error message should contain actionable hints
|
||||
errMsg := err.Error()
|
||||
if !strings.Contains(errMsg, "moveTaskPosition") {
|
||||
t.Error("error should mention operation name")
|
||||
}
|
||||
if !strings.Contains(errMsg, "possible causes") {
|
||||
t.Error("error should include possible causes")
|
||||
}
|
||||
}
|
||||
|
||||
func TestClient_MoveTaskToProject(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue