feat!: add support for Echo v5#1183
Conversation
Semver Impact of This PR🔴 Major (breaking changes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Breaking Changes 🛠
New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Deps
Other
🤖 This preview updates automatically when you update the PR. |
|
Hey @Scorfly, we can't really support this right now, since the module versioning needs to be consistent across packages, and we also want to support the three latest go versions. |
|
This is quite unprecedented. Upgrading to echo/v5 while using Sentry is causing problems. |
|
We already had some discussions internally about upgrading go versions for the integrations separately to the main sdk, so we will support v5. |
|
Hey @Scorfly, I updated the ci to allow bumping echo to 1.25. You can update the branch and we can merge the change. |
|
Thx ! |
18eefef to
1a3f0e9
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1183 +/- ##
==========================================
- Coverage 86.04% 82.28% -3.76%
==========================================
Files 62 75 +13
Lines 6090 7272 +1182
==========================================
+ Hits 5240 5984 +744
- Misses 635 1018 +383
- Partials 215 270 +55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| defer func() { | ||
| status := ctx.Response().Status | ||
| var status int | ||
| if resp, err := echo.UnwrapResponse(ctx.Response()); err == nil && resp.Status != 0 { |
There was a problem hiding this comment.
We should skip setting http.response.status_code if Status = 0. Also let's add a debug statement that we couldn't get the response status code.
|
Overall looks good, let's just amend the comment. Just FYI, we will release this after 31-03, as per the recommendation here |
1a3f0e9 to
f8d2c97
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
BREAKING CHANGE: echo integration now requires github.com/labstack/echo/v5
and Go 1.25. Echo v4 is no longer supported.
- Bump github.com/labstack/echo from v4.10.1 to v5.0.0.
- Set go directive to 1.25.0 in echo/go.mod.
- Drop Echo v4-only indirects (gommon, go-colorable, go-isatty,
bytebufferpool, fasttemplate, x/crypto, x/net) and update remaining
(x/sys, x/text). go.sum updated accordingly.
- Use *echo.Context instead of echo.Context in handler and public API
(GetHubFromContext, SetHubOnContext, GetSpanFromContext) to match
v5’s pointer-based Context.
- Update all handler signatures in code and docs from
`func(c echo.Context) error` to `func(c *echo.Context) error`.
- In v5, Context.Response() returns http.ResponseWriter, not *Response,
so ctx.Response().Status is no longer available.
- Replace direct ctx.Response().Status with echo.UnwrapResponse(ctx.Response())
to obtain *echo.Response and use resp.Status when UnwrapResponse succeeds
and resp.Status != 0.
- When UnwrapResponse fails (e.g. middleware replaces the response with a
writer that does not unwrap to *echo.Response), leave status at its zero
value (0) instead of defaulting to 200.
- For handler-returned errors, use echo.HTTPStatusCoder instead of
*echo.HTTPError so that both *HTTPError and unexported *httpError
(ErrNotFound, ErrMethodNotAllowed, etc.) are handled and the correct
status is used for the transaction.
- In TestIntegration, skip route registration when Handler is nil so the
“404 / no route” case does not call router.GET("", nil). Echo v5’s
router rejects Handler == nil and panics with “adding route without
handler function”.
- Add TestUnwrapResponseError: when the response is wrapped by a writer
that does not implement Unwrap() (e.g. &struct{ http.ResponseWriter }{}),
UnwrapResponse returns an error; the middleware must not panic and must
record http.response.status_code as 0 in the transaction.
- README.md and example_test.go: switch imports and examples from
echo/v4 to echo/v5 and from echo.Context to *echo.Context.
f8d2c97 to
535b494
Compare
|
What are the plans for cutting a release with this? We're eagerly awaiting a new version to be published and I wanted to avoid pinning a specific commit on the master branch :) Thanks! |
|
I want to also include one more thing for the release, but will happen this or the next week. |
BREAKING CHANGE: echo integration now requires github.com/labstack/echo/v5 and Go 1.25. Echo v4 is no longer supported.
Dependency and module
Echo v5 API adjustments
*echo.Contextinstead ofecho.Contextin handler and public API (GetHubFromContext, SetHubOnContext, GetSpanFromContext) to match v5’s pointer-based Context.func(c echo.Context) errortofunc(c *echo.Context) error.Context.Response()returnshttp.ResponseWriter, not*Response, soctx.Response().Statusis no longer available.HTTP status and Response handling
ctx.Response().Status withecho.UnwrapResponse(ctx.Response())to obtain*echo.Responseand useresp.StatuswhenUnwrapResponsesucceeds andresp.Status != 0.UnwrapResponsefails (e.g. middleware replaces the response with a writer that does not unwrap to*echo.Response), leave status at its zero value (0) instead of defaulting to 200.echo.HTTPStatusCoderinstead of*echo.HTTPErrorso that both*HTTPErrorand unexported*httpError(ErrNotFound, ErrMethodNotAllowed, etc.) are handled and the correct status is used for the transaction.Tests
router.GET("", nil). Echo v5’s router rejectsHandler == niland panics with “adding route without handler function”.TestUnwrapResponseError: when the response is wrapped by a writer that does not implementUnwrap()(e.g.&struct{ http.ResponseWriter }{}), UnwrapResponse returns an error; the middleware must not panic and must recordhttp.response.status_codeas 0 in the transaction.Documentation and examples
README.mdandexample_test.go: switch imports and examples from echo/v4 to echo/v5 and fromecho.Contextto*echo.Context.