Skip to content

build(actix): Update actix-web to v4. #316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

fourbytes
Copy link
Contributor

Updated sentry-actix to support actix-web v4 (currently still in beta).

Copy link
Contributor

@aramperes aramperes left a comment

Choose a reason for hiding this comment

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

Note: this raises the minimum supported Rust version from 1.45 to 1.46: actix/actix-web#1858

The Test MSRV job will need to be updated:

rust: [1.45.0]

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

lgtm! lints are still failing, and I will wait with merging this until we have a stable release.

@jan-auer jan-auer changed the title deps(sentry-actix): updated actix to v4. build(actix): Update actix-web to v4. Mar 3, 2021
@@ -13,7 +13,7 @@ edition = "2018"

[dependencies]
sentry-core = { version = "0.22.0", path = "../sentry-core", default-features = false }
Copy link

@JakubKoralewski JakubKoralewski Jul 7, 2021

Choose a reason for hiding this comment

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

I need to add this for my project using actix-web v4 and this sentry-actix branch to compile for some reason:

Suggested change
sentry-core = { version = "0.22.0", path = "../sentry-core", default-features = false }
sentry-core = { version = "0.22.0", path = "../sentry-core", default-features = false, features=["client"] }

Choose a reason for hiding this comment

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

I also had to do this in my local fork

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, not sure about this as it seems to be working in my project and wasn't needed in previous versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, just managed to reproduce this. Honestly not sure why it wasn't happening before. It does appear it is now necessary though so I've updated the PR.

Copy link

@mlodato517 mlodato517 left a comment

Choose a reason for hiding this comment

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

I think this needs to update the tests to remove the &muts in call_service which now takes a shared reference.

@fourbytes fourbytes marked this pull request as ready for review July 17, 2021 09:09
@fourbytes
Copy link
Contributor Author

Latest update should fix those lints, but happy to wait for the full release of v4 for this to be merged.

@ppamorim
Copy link

@fourbytes Any update on this, I can't use it because I am getting this error:

the trait `Transform<actix_web::app_service::AppRouting, ServiceRequest>` is not implemented for `Sentry`

This error is similar to the one I found here: actix/actix-web#2633

@Swatinem
Copy link
Member

superceded by #437

@Swatinem Swatinem closed this Feb 28, 2022
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.

6 participants