clone/vikunja
1
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-04-29 16:36:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: centralize HTTP error handling (#2062)

Browse Source 
This changes the error handling to a centralized HTTP error handler in `pkg/routes/error_handler.go` that converts all error types to proper HTTP responses. This simplifies the overall error handling because http handler now only need to return the error instead of calling HandleHTTPError as previously.
It also removes the duplication between handling errors with and without Sentry.

🐰 Hop along, dear errors, no more wrapping today!
We've centralized handlers in a shiny new way,
From scattered to unified, the code flows so clean,
ValidationHTTPError marshals JSON supreme!
Direct propagation hops forward with glee,
A refactor so grand—what a sight to see! 🎉
This commit is contained in:
kolaente
2026-01-08 11:02:59 +01:00
committed by GitHub
parent 4f31300915
commit 39b4568bc5
42 changed files with 750 additions and 447 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
pkg/webtests/task_comment_test.go
View File

@@ -23,7 +23,6 @@ import (
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/api/pkg/web/handler"
"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -68,70 +67,70 @@ func TestTaskComments(t *testing.T) {
t.Run("Forbidden", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "14", "commentid": "2"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team readonly", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "15", "commentid": "3"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team write", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "16", "commentid": "4"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team admin", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "17", "commentid": "5"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User readonly", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "18", "commentid": "6"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User write", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "19", "commentid": "7"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User admin", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "20", "commentid": "8"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team readonly", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "21", "commentid": "9"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team write", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "22", "commentid": "10"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team admin", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "23", "commentid": "11"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User readonly", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "24", "commentid": "12"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User write", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "25", "commentid": "13"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User admin", func(t *testing.T) {
_, err := testHandler.testUpdateWithUser(nil, map[string]string{"task": "26", "commentid": "14"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
})
})
@@ -151,70 +150,70 @@ func TestTaskComments(t *testing.T) {
t.Run("Forbidden", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "14", "commentid": "2"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team readonly", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "15", "commentid": "3"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team write", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "16", "commentid": "4"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team admin", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "17", "commentid": "5"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User readonly", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "18", "commentid": "6"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User write", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "19", "commentid": "7"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User admin", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "20", "commentid": "8"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team readonly", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "21", "commentid": "9"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team write", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "22", "commentid": "10"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team admin", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "23", "commentid": "11"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User readonly", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "24", "commentid": "12"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User write", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "25", "commentid": "13"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User admin", func(t *testing.T) {
_, err := testHandler.testDeleteWithUser(nil, map[string]string{"task": "26", "commentid": "14"})
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
})
})
@@ -234,12 +233,12 @@ func TestTaskComments(t *testing.T) {
// Owned by user13
_, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "34"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team readonly", func(t *testing.T) {
_, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "15"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Team write", func(t *testing.T) {
rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "16"}, `{"comment":"Lorem Ipsum"}`)
@@ -255,7 +254,7 @@ func TestTaskComments(t *testing.T) {
t.Run("Shared Via User readonly", func(t *testing.T) {
_, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "18"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via User write", func(t *testing.T) {
rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "19"}, `{"comment":"Lorem Ipsum"}`)
@@ -271,7 +270,7 @@ func TestTaskComments(t *testing.T) {
t.Run("Shared Via Parent Project Team readonly", func(t *testing.T) {
_, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "21"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project Team write", func(t *testing.T) {
rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "22"}, `{"comment":"Lorem Ipsum"}`)
@@ -287,7 +286,7 @@ func TestTaskComments(t *testing.T) {
t.Run("Shared Via Parent Project User readonly", func(t *testing.T) {
_, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "24"}, `{"comment":"Lorem Ipsum"}`)
require.Error(t, err)
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`)
})
t.Run("Shared Via Parent Project User write", func(t *testing.T) {
rec, err := testHandler.testCreateWithUser(nil, map[string]string{"task": "25"}, `{"comment":"Lorem Ipsum"}`)