diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 1bf8d23..445a5f7 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -35,11 +35,116 @@ jobs: - name: Run migrations run: goose -dir .sqlc/migrations postgres "postgres://postgres:postgres@localhost:5432/postgres?sslmode=disable" up - name: Run tests - run: go test -v ./... - # Comment test results on the PR + id: tests + continue-on-error: true + run: | + # run tests and capture output + go test -v -coverprofile=coverage.out ./... 2>&1 | tee test_output.txt + # store the exit code explicitly + echo "::set-output name=exit_code::${PIPESTATUS[0]}" + - name: Generate coverage report + run: go tool cover -html=coverage.out -o coverage.html + - name: Upload coverage report + if: ${{ !env.ACT && github.event_name == 'pull_request' }} + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: coverage.html - name: Comment PR - if: github.event_name == 'pull_request' && always() + if: ${{ !env.ACT && github.event_name == 'pull_request' && always() }} uses: actions/github-script@v7 with: github-token: ${{ secrets.GITHUB_TOKEN }} - script: "const output = `#### Test Results\nTests: ${{ job.status }}\n\n*Workflow triggered by @${{ github.actor }}, ran on \\`${{ runner.os }}\\`*`;\n \ngithub.rest.issues.createComment({\n issue_number: context.issue.number,\n owner: context.repo.owner,\n repo: context.repo.repo,\n body: output\n})\n" + script: | + const fs = require('fs'); + + // Read test output + const testOutput = fs.readFileSync('test_output.txt', 'utf8'); + + // Get coverage - look for the last coverage number in the output + let coverage = 'N/A'; + const coverageMatches = testOutput.match(/coverage: (\d+\.\d+)% of statements/g) || []; + if (coverageMatches.length > 0) { + const lastMatch = coverageMatches[coverageMatches.length - 1]; + coverage = lastMatch.match(/(\d+\.\d+)%/)[1] + '%'; + } + + // Check if any tests failed + const hasFailed = testOutput.includes('FAIL') && !testOutput.includes('FAIL\t[build failed]'); + const testStatus = hasFailed ? 'failure' : 'success'; + const color = testStatus === 'success' ? '✅' : '❌'; + + // Parse test failures + let failureDetails = ''; + if (hasFailed) { + const errorTraces = testOutput.match(/\s+.*?_test\.go:\d+:[\s\S]*?Test:\s+.*$/gm) || []; + const failures = testOutput.match(/--- FAIL: .*?(?=(?:---|\z))/gs) || []; + + failureDetails = ` +
+ ❌ Test Failures + + \`\`\` + ${failures.join('\n')} + + Error Details: + ${errorTraces.map(trace => trace.trim()).join('\n')} + \`\`\` +
+ `; + } + + const output = `### Test Results ${color} + + **Status**: ${testStatus} + **Coverage**: ${coverage} + **OS**: \`${{ runner.os }}\` + + ${failureDetails} + +
+ Test Details + + * Triggered by: @${{ github.actor }} + * Commit: ${{ github.sha }} + * Branch: ${{ github.ref }} + * Workflow: ${{ github.workflow }} +
`; + + // Find existing comment + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + + const botComment = comments.find(comment => + comment.user.type === 'Bot' && + comment.body.includes('### Test Results') + ); + + if (botComment) { + // Update existing comment + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: botComment.id, + body: output + }); + } else { + // Create new comment + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: output + }); + } + - uses: actions/cache@v4 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- diff --git a/internal/server/error_handler.go b/internal/server/error_handler.go new file mode 100644 index 0000000..2bd49cd --- /dev/null +++ b/internal/server/error_handler.go @@ -0,0 +1,79 @@ +package server + +import ( + "net/http" + + "github.com/go-playground/validator/v10" + "github.com/labstack/echo/v4" + "github.com/rs/zerolog/log" +) + +type ErrorResponse struct { + Status int `json:"status"` + Message string `json:"message"` + RequestID string `json:"request_id,omitempty"` + Errors []string `json:"errors,omitempty"` +} + +func globalErrorHandler(err error, c echo.Context) { + req := c.Request() + requestID := req.Header.Get(echo.HeaderXRequestID) + + // default error response + status := http.StatusInternalServerError + message := "internal server error" + var validationErrors []string + + // handle different error types + switch e := err.(type) { + case *echo.HTTPError: + status = e.Code + message = e.Message.(string) + + case validator.ValidationErrors: + // handle validation errors specially + status = http.StatusBadRequest + message = "validation failed" + validationErrors = make([]string, len(e)) + for i, err := range e { + validationErrors[i] = err.Error() + } + + case error: + message = e.Error() + } + + // log with more context + logger := log.Error() + if status < 500 { + logger = log.Warn() + } + + logger. + Err(err). + Str("request_id", requestID). + Str("method", req.Method). + Str("path", req.URL.Path). + Int("status", status). + Str("user_agent", req.UserAgent()). + Msg("request error") + + // return json response + if !c.Response().Committed { + response := ErrorResponse{ + Status: status, + Message: message, + RequestID: requestID, + } + if len(validationErrors) > 0 { + response.Errors = validationErrors + } + + if err := c.JSON(status, response); err != nil { + log.Error(). + Err(err). + Str("request_id", requestID). + Msg("failed to send error response") + } + } +} diff --git a/internal/server/error_handler_test.go b/internal/server/error_handler_test.go new file mode 100644 index 0000000..7544de9 --- /dev/null +++ b/internal/server/error_handler_test.go @@ -0,0 +1,94 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-playground/validator/v10" + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" +) + +func TestGlobalErrorHandler(t *testing.T) { + // setup + e := echo.New() + e.HTTPErrorHandler = globalErrorHandler + + // test cases + tests := []struct { + name string + handler echo.HandlerFunc + expectedStatus int + expectedBody string + }{ + { + name: "http error", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusBadRequest, "bad request") + }, + expectedStatus: http.StatusBadRequest, + expectedBody: `{"status":400,"message":"bad request"}`, + }, + { + name: "generic error", + handler: func(c echo.Context) error { + return echo.NewHTTPError(http.StatusInternalServerError, "something went wrong") + }, + expectedStatus: http.StatusInternalServerError, + expectedBody: `{"status":500,"message":"something went wrong"}`, + }, + { + name: "validation error", + handler: func(c echo.Context) error { + type TestStruct struct { + Email string `validate:"required,email"` + Age int `validate:"required,gt=0"` + } + + v := validator.New() + err := v.Struct(TestStruct{ + Email: "invalid-email", + Age: -1, + }) + + return err + }, + expectedStatus: http.StatusBadRequest, + expectedBody: `{ + "status": 400, + "message": "validation failed", + "errors": [ + "Key: 'TestStruct.Email' Error:Field validation for 'Email' failed on the 'email' tag", + "Key: 'TestStruct.Age' Error:Field validation for 'Age' failed on the 'gt' tag" + ] + }`, + }, + { + name: "with request id", + handler: func(c echo.Context) error { + c.Request().Header.Set(echo.HeaderXRequestID, "test-123") + return echo.NewHTTPError(http.StatusBadRequest, "bad request") + }, + expectedStatus: http.StatusBadRequest, + expectedBody: `{"status":400,"message":"bad request","request_id":"test-123"}`, + }, + } + + // run tests + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + err := tt.handler(c) + if err != nil { + e.HTTPErrorHandler(err, c) + } + + assert.Equal(t, tt.expectedStatus, rec.Code) + assert.JSONEq(t, tt.expectedBody, rec.Body.String()) + }) + } +} diff --git a/internal/server/index.go b/internal/server/index.go index 888304f..22e160f 100644 --- a/internal/server/index.go +++ b/internal/server/index.go @@ -63,7 +63,8 @@ func New(testing bool) (*Server, error) { ) } - // setup middlewares + // setup error handler and middlewares + e.HTTPErrorHandler = globalErrorHandler e.Use(middleware.Logger()) e.Use(echoMiddleware.Recover()) e.Use(apiLimiter.RateLimit()) // global rate limit