Skip to content

Commit

Permalink
Merge branch 'main' into 45-be-implement-rate-limiting-for-api-endpoints
Browse files Browse the repository at this point in the history
  • Loading branch information
AmirAgassi authored Nov 12, 2024
2 parents cde91ba + 60165e3 commit 63c8259
Show file tree
Hide file tree
Showing 4 changed files with 284 additions and 5 deletions.
113 changes: 109 additions & 4 deletions .github/workflows/run-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = `
<details>
<summary>❌ Test Failures</summary>
\`\`\`
${failures.join('\n')}
Error Details:
${errorTraces.map(trace => trace.trim()).join('\n')}
\`\`\`
</details>
`;
}
const output = `### Test Results ${color}
**Status**: ${testStatus}
**Coverage**: ${coverage}
**OS**: \`${{ runner.os }}\`
${failureDetails}
<details>
<summary>Test Details</summary>
* Triggered by: @${{ github.actor }}
* Commit: ${{ github.sha }}
* Branch: ${{ github.ref }}
* Workflow: ${{ github.workflow }}
</details>`;
// 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-
79 changes: 79 additions & 0 deletions internal/server/error_handler.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
}
94 changes: 94 additions & 0 deletions internal/server/error_handler_test.go
Original file line number Diff line number Diff line change
@@ -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())
})
}
}
3 changes: 2 additions & 1 deletion internal/server/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 63c8259

Please sign in to comment.