404 response not returned when expected about syncstorage-rs HOT 5 CLOSED

Fwiw, I spent a bit of time trying to fix this today, but failed to bend Actix to my will. Essentially I was hoping to use App::default_resource like so:

diff --git a/src/server/mod.rs b/src/server/mod.rs
index 0530100..fb60c6e 100644
--- a/src/server/mod.rs
+++ b/src/server/mod.rs
@@ -74,7 +74,11 @@ pub fn build_app(state: ServerState) -> App<ServerState> {
         .middleware(middleware::WeaveTimestamp)
         .middleware(middleware::DbTransaction)
         .middleware(middleware::PreConditionCheck)
-        .configure(|app| init_routes!(Cors::for_app(app)).register())
+        .configure(|app| {
+            init_routes!(Cors::for_app(app))
+                .register()
+                .default_resource(|r| r.f(handlers::not_found))
+        })
 }

 pub struct Server {}
diff --git a/src/web/handlers.rs b/src/web/handlers.rs
index 685f831..060579e 100644
--- a/src/web/handlers.rs
+++ b/src/web/handlers.rs
@@ -1,7 +1,7 @@
 //! API Handlers
 use std::collections::HashMap;

-use actix_web::{http::StatusCode, FutureResponse, HttpResponse, State};
+use actix_web::{http::StatusCode, FutureResponse, HttpRequest, HttpResponse, State};
 use futures::future::{self, Either, Future};
 use serde::Serialize;

@@ -368,3 +368,9 @@ pub fn get_configuration(
 ) -> FutureResponse<HttpResponse> {
     Box::new(future::result(Ok(HttpResponse::Ok().json(&*state.limits))))
 }
+
+pub fn not_found(req: &HttpRequest<ServerState>) -> HttpResponse {
+    req.build_response(StatusCode::NOT_FOUND)
+        .content_type("application/json")
+        .body("0")
+}

Not sure if the cors stuff is affecting it or maybe I've misunderstood the docs/example. Anyway, just recording what I tried, I can come back to this later (want to try and achieve something tangible before today's meeting, lol).

Branch: pb/115

from syncstorage-rs.

@philbooth tried out this branch against the e2e test w/ RUST_LOG=actix_web=debug: which spit out actix_web::pipeline: Error occured during request handling: missing field `uid` caused by HawkIdentifier::from_request in the PreconditionCheck response.

So errors from middlewares short circuit the request handling.

2019-06-04 edit: RUSTLOG -> RUST_LOG

from syncstorage-rs.

Looking at latest master:

GET /some/url => 404
GET /1.5/some/url => 401

Is this still a valid bug? Do we want to do a resource check before authorization (I'd think we'd want to know the request is valid before we do a resource check)?

from syncstorage-rs.

The test still fails (still a valid issue) but it's probably the least important of the failing e2e tests.

Let's hold off on this one for now, as it's dependent on actix-web's middleware behavior which significantly changes in 1.0.

The current middlewares could either be more careful to avoid error responses in cases like this or possibly they could check the request to determine if the route had a specific match (if no match, don't do anything, we're 404ing anyway).

..but let's wait for 1.0 to have a chance to settle

from syncstorage-rs.

This was fixed during the actix-web 1.0 upgrade #126

from syncstorage-rs.