Skip to content

Feature implementation from commits 1f1bd19..d756ec1#3

Open
codeOwlAI wants to merge 15 commits into
feature-base-branch-3from
feature-head-branch-3
Open

Feature implementation from commits 1f1bd19..d756ec1#3
codeOwlAI wants to merge 15 commits into
feature-base-branch-3from
feature-head-branch-3

Conversation

@codeOwlAI
Copy link
Copy Markdown
Owner

@codeOwlAI codeOwlAI commented Jun 30, 2025

PR Summary

Context Interface Implementation and Error Handling Improvements

Overview

This PR implements the standard Go context.Context interface in Fiber's Ctx type, improves error handling across multiple components, and fixes several issues with the proxy module and test reliability.

Change Types

Type Description
Enhancement Implementation of context.Context interface in Fiber's Ctx type
Bugfix Improved error handling in various functions
Refactor Changed TlsConfig to TLSConfig for consistent capitalization
Test Replaced external dependencies with local test servers
Style Fixed typos and grammatical errors in comments

Affected Modules

Module / File Change Description
ctx.go Added context.Context interface methods (Err, Value, etc.)
helpers.go Refactored genericParseType to return errors instead of panicking
proxy/config.go Renamed TlsConfig to TLSConfig for consistent capitalization
proxy/proxy_test.go Replaced external URL dependencies with local test servers
timeout/timeout.go Changed context implementation affecting timeout functionality
ctx_test.go Added tests for context.Context interface methods

Breaking Changes

  • Refactored genericParseType function now returns errors instead of using panic or default values
  • Changed context implementation in timeout module breaks timeout functionality
  • Renamed field from 'TlsConfig' to 'TLSConfig' in Config struct (planned v3 change)

Notes for Reviewers

  • The timeout module changes appear to break functionality and should be carefully reviewed
  • The error handling changes in genericParseType represent a breaking API change
  • The context.Context interface implementation is a significant enhancement that should be thoroughly tested

gaby and others added 15 commits May 21, 2025 14:34
* Expand Test_Utils_Parse_Address

* Update implementation based on Codex

* Remove duplicated func

* Update test fmt

* format

* Update helpers_test.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* more format

* more format

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…#3467)

* Fix proxy middleware tests to avoid external network

* Update proxy_test.go

* Update middleware/proxy/proxy_test.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update proxy_test.go

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ofiber#3440)

* build(deps): bump github.com/valyala/fasthttp from 1.60.0 to 1.62.0

Bumps [github.com/valyala/fasthttp](https://github.com/valyala/fasthttp) from 1.60.0 to 1.62.0.
- [Release notes](https://github.com/valyala/fasthttp/releases)
- [Commits](valyala/fasthttp@v1.60.0...v1.62.0)

---
updated-dependencies:
- dependency-name: github.com/valyala/fasthttp
  dependency-version: 1.62.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Fix CSRF middleware tests for fasthttp 1.62 (gofiber#3471)

Fix CSRF tests for fasthttp 1.62

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RW <rene@gofiber.io>
adpater / HTTPHandler
NEW
Benchmark_HTTPHandler-12    	 1762837	       640.6 ns/op	     696 B/op	      10 allocs/op
Benchmark_HTTPHandler-12    	 1924524	       616.5 ns/op	     696 B/op	      10 allocs/op
Benchmark_HTTPHandler-12    	 1838780	       650.4 ns/op	     696 B/op	      10 allocs/op
Benchmark_HTTPHandler-12    	 1876947	       644.0 ns/op	     696 B/op	      10 allocs/op
OLD
Benchmark_HTTPHandler-12    	 1864819	       667.2 ns/op	     720 B/op	      11 allocs/op
Benchmark_HTTPHandler-12    	 1892569	       677.0 ns/op	     720 B/op	      11 allocs/op
Benchmark_HTTPHandler-12    	 1811704	       639.5 ns/op	     720 B/op	      11 allocs/op
Benchmark_HTTPHandler-12    	 1879849	       644.0 ns/op	     720 B/op	      11 allocs/op

Utils / IsNoCache
NEW
Benchmark_Utils_IsNoCache-12    	44307204	        27.08 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_IsNoCache-12    	40782919	        26.88 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_IsNoCache-12    	44228217	        26.69 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_IsNoCache-12    	45605700	        26.75 ns/op	       0 B/op	       0 allocs/op
OLD
Benchmark_Utils_IsNoCache-12    	30043908	        37.80 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_IsNoCache-12    	32137476	        37.51 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_IsNoCache-12    	31474653	        37.92 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_IsNoCache-12    	31838683	        37.71 ns/op	       0 B/op	       0 allocs/op
* ♻️ refact: make genericParseType return error

* 🐛 fix: return error when parsing unsupported type

* 🚨 test: cover the default value for Params

* 🚨 test: cover default value on parsing error

* ♻️ refact: change the benchmark name

* 🚨 test: remove the duplicated maxUint16 test case

---------

Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com>
* Ctx implements context.Context

* fix up some linting issues

* added some tests

* no message

* fiber.Ctx implements context.Context

* no message

* implement compile-time check

* update formatting

* update compile-time checks

---------

Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com>
// Create a context with the specified timeout; any operation exceeding
// this deadline will be canceled automatically.
timeoutContext, cancel := context.WithTimeout(ctx.Context(), timeout)
timeoutContext, cancel := context.WithTimeout(ctx, timeout)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Context Type Mismatch.

The code incorrectly passes a Fiber context object directly to WithTimeout instead of using ctx.Context(), breaking timeout functionality.

Current Code (Diff):

-		timeoutContext, cancel := context.WithTimeout(ctx, timeout)
+		timeoutContext, cancel := context.WithTimeout(ctx.Context(), timeout)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
timeoutContext, cancel := context.WithTimeout(ctx, timeout)
timeoutContext, cancel := context.WithTimeout(ctx.Context(), timeout)

app.Get("/fast", New(func(c fiber.Ctx) error {
// Simulate some work
if err := sleepWithContext(c.Context(), 10*time.Millisecond, context.DeadlineExceeded); err != nil {
if err := sleepWithContext(c, 10*time.Millisecond, context.DeadlineExceeded); err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Type Mismatch: Incorrect Context Parameter.

The function sleepWithContext expects a context.Context but is now receiving a fiber.Ctx object which may not implement the context.Context interface.

Current Code (Diff):

- 		if err := sleepWithContext(c, 10*time.Millisecond, context.DeadlineExceeded); err != nil {
+ 		if err := sleepWithContext(c.Context(), 10*time.Millisecond, context.DeadlineExceeded); err != nil {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
if err := sleepWithContext(c, 10*time.Millisecond, context.DeadlineExceeded); err != nil {
if err := sleepWithContext(c.Context(), 10*time.Millisecond, context.DeadlineExceeded); err != nil {

// This handler sleeps 200ms, exceeding the 100ms limit.
app.Get("/slow", New(func(c fiber.Ctx) error {
if err := sleepWithContext(c.Context(), 200*time.Millisecond, context.DeadlineExceeded); err != nil {
if err := sleepWithContext(c, 200*time.Millisecond, context.DeadlineExceeded); err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Type mismatch in function call.

The code passes a fiber.Ctx object directly to sleepWithContext which expects a context.Context, potentially causing a compilation failure.

Current Code (Diff):

- 		if err := sleepWithContext(c, 200*time.Millisecond, context.DeadlineExceeded); err != nil {
+ 		if err := sleepWithContext(c.Context(), 200*time.Millisecond, context.DeadlineExceeded); err != nil {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
if err := sleepWithContext(c, 200*time.Millisecond, context.DeadlineExceeded); err != nil {
if err := sleepWithContext(c.Context(), 200*time.Millisecond, context.DeadlineExceeded); err != nil {

🔄 Dependencies Affected

middleware

Function: sleepWithContext

Issue: Function expects context.Context as first parameter but receives fiber.Ctx

Suggestion: Ensure the correct context type is passed from the caller

Proposed Code:

func sleepWithContext(ctx context.Context, d time.Duration, te error) error {
  // Function expects context.Context type
}

// Sleep might time out, or might return early. If the context is canceled,
// we treat errCustomTimeout as a 'timeout-like' condition.
if err := sleepWithContext(c.Context(), 200*time.Millisecond, errCustomTimeout); err != nil {
if err := sleepWithContext(c, 200*time.Millisecond, errCustomTimeout); err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Type mismatch in function parameter.

The sleepWithContext() function expects context.Context but is being passed a fiber.Ctx object which will cause compilation errors.

Current Code (Diff):

- 		if err := sleepWithContext(c, 200*time.Millisecond, errCustomTimeout); err != nil {
+ 		if err := sleepWithContext(c.Context(), 200*time.Millisecond, errCustomTimeout); err != nil {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
if err := sleepWithContext(c, 200*time.Millisecond, errCustomTimeout); err != nil {
if err := sleepWithContext(c.Context(), 200*time.Millisecond, errCustomTimeout); err != nil {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants