ref: Remove actix-web in favor of axum (NATIVE-193) #544
Conversation
| use axum::http::{Response, StatusCode}; | ||
| use axum::response::IntoResponse; | ||
| use axum::Router; | ||
| use sentry::integrations::tower::NewSentryLayer; |
There was a problem hiding this comment.
This was done recently.
getsentry/sentry-rust#356
It is the reason why this pulls in sentry-rust via git. I think we should be able to cut a release for that any time.
| pub use error::ResponseError; | ||
|
|
||
| use applecrashreport::handle_apple_crash_report_request as applecrashreport; | ||
| use minidump::handle_minidump_request as minidump; | ||
| use proxy::proxy_symstore_request as proxy; | ||
| use requests::poll_request as requests; | ||
| use symbolicate::symbolicate_frames as symbolicate; |
There was a problem hiding this comment.
nitpicking: Isn't this section above the mod section normally? Also the pub use one should be the bottom of this section afaik https://www.notion.so/sentry/HOWTO-Code-Rust-at-Sentry-7b35f165b10b4492bb95ebe1471a9ada#57304f11240944338fe3828059bcf463
| } | ||
|
|
||
| impl<F> SentryFutureExt for F where F: futures01::future::Future {} | ||
|
|
||
| /// Write own data to [`sentry::Scope`], only the subset that is considered useful for debugging. | ||
| pub trait ConfigureScope { |
There was a problem hiding this comment.
lol, I've never liked this trait and wouldn't mind if this goes away too. but doesn't matter for this PR
|
I'm kind of in favour of still resolving the two unticked issues in the description. |
Implement file streaming and limits on multipart filesizes
| } | ||
|
|
||
| fn call(&mut self, request: Request<Body>) -> Self::Future { | ||
| sentry::configure_scope(|scope| { |
There was a problem hiding this comment.
Shouldn't we turn the future returned from self.service.call() into a SentryFuture and use with_scope instead so that this is all popped back off the scope when this is done?
There was a problem hiding this comment.
The sentry-tower layer makes sure that every request gets its own fresh hub and scope, and if I understand the tower concepts correctly, all child layers/routes get the correct hub.
| fn get_str(val: &HeaderValue) -> Option<&str> { | ||
| val.to_str().ok() | ||
| } |
There was a problem hiding this comment.
heh, i would never have thought of writing this as an fn rather than a closure. but that makes total sense.
|
I filed that as https://getsentry.atlassian.net/browse/NATIVE-207 as a separate backlog task so it does not block us from marking the parent task as "done". |
The 5MB limit to "/symbolicate" bodies was removed in #544, which was a major refactor. This restores that limit.
Still needs a bit of work, there are a few TODOs scattered around, but it already gets rid of old actix/futures/tokio.
There are a few changes from the current actix implementation:
ApiResponseErroror however it is called, this might be good to do as a followup refactor.