WIP: upgrade to axum 0.8 #2705
WIP: upgrade to axum 0.8 #2705
Conversation
github-actions
bot
added
the
S-waiting-on-review
src/web/sitemap.rs
Outdated
| @@ -154,6 +154,7 @@ about_page!(AboutPageMetadata, "core/about/metadata.html"); | |||
| about_page!(AboutPageRedirection, "core/about/redirections.html"); | |||
| about_page!(AboutPageDownload, "core/about/download.html"); | |||
|
|
|||
| #[axum::debug_handler] | |||
| pub(crate) async fn about_handler(subpage: Option<Path<String>>) -> AxumResult<impl IntoResponse> { | |||
There was a problem hiding this comment.
out of curiosity: wouldn't it be more straight-forward to have the subpages be part of the router and have dedicated handlers for them? :)
There was a problem hiding this comment.
probably :)
this is historically coming the old iron routes, and I didn't change how they were structured when migrating to axum.
( the list of possible refactors is very long 😄 , I'm trying to make progress as much as possible )
src/web/mod.rs
Outdated
| @@ -413,7 +413,9 @@ async fn apply_middleware( | |||
| Ok(router.layer( | |||
| ServiceBuilder::new() | |||
| .layer(TraceLayer::new_for_http()) | |||
| .layer(sentry_tower::NewSentryLayer::new_from_top()) | |||
| // FIXME: send / sync error? | |||
There was a problem hiding this comment.
I'm not sure why this breaks, my guess is that sentry-tower is the problem, because its SentryLayer contains _hub: PhantomData<(H, Request)>, where the axum request is not Sync.
I might be wrong, but will have to do more digging on this. If I'm right then Crates.io will likely also run into this same issue, will also see how they fix it
There was a problem hiding this comment.
@Turbo87 you might run into the same issue on crates.io, my guess is that the solution will be the fix in sentry-tower:
There was a problem hiding this comment.
Interesting. We're using the MatchedPath sentry extension for axum which is still using the old version, which is why I hadn't updated yet. I was assuming that that would be all that needs updating 😅
There was a problem hiding this comment.
I didn't realize sentry-tower implemented this :)
( I built a small middleware for the same thing)
Interesting. We're using the MatchedPath sentry extension for axum which is still using the old version, which is why I hadn't updated yet. I was assuming that that would be all that needs updating 😅
My guess is that it will break the same way :)
There was a problem hiding this comment.
It did after I implemented it a while ago :D
| .nest("/-/static/", build_static_router()) | ||
| // `.nest` with fallbacks is currently broken, `.nest_service works | ||
| // https://github.com/tokio-rs/axum/issues/3138 | ||
| .nest_service("/-/static", build_static_router()) |
There was a problem hiding this comment.
@Turbo87 also you might run into this one when working on crates.io :)
( not sure how you are using nesting?)
There was a problem hiding this comment.
I think we're not using nesting and provide static assets via middleware, but we'll see...
I believe there is an open bug with axum blocking this upgrade: tokio-rs/axum#3138there is a workaround for this bug, which I already added (using.nest_serviceinstead of.nest)