ktoso
commented
8 years ago Open ktoso opened 8 years ago
ktoso
commented
8 years ago 
not sure what to make of this: should this be regarded as “triaged”, and if yes on which milestone?
ktoso
commented
8 years ago 
It's something that I'd keep as a general improvement point for the marshalling/unmarshalling infrastructure and as something that users can hang their hat onto, if they experience the problem. My take is that it's not a high prio issue that we really need to schedule onto any particular milestone at this point.
ktoso
commented
8 years ago ktoso
commented
8 years ago 
Esp. with content negotiation and rendering of Source for different content types this is even worse than just rendering a strict thing.
Also, users noticing and being very suprised by it: https://github.com/akka/akka/issues/18625#issuecomment-198998788
I hope it's not too late to fix in a BC way hm.
Ah, it's in akka-http, uff, so we have time to fix it even if not BC.
ktoso
commented
8 years ago 
for unsafe methods this could actually result in a state change that doesn't get signaled properly to the client
This is pretty major as it's unexpected behavior with side-effects, especially with respondWithMediaType not migrated from spray to akka-http.
Of course proper testing (like that which raised the issue on my end) will catch this, but this is still a bad default.
Also, users noticing and being very suprised by it: https://github.com/akka/akka/issues/18625#issuecomment-198998788
Hi :)
ktoso
commented
8 years ago
If I understand the problem correctly I don’t see why this is connected to the marshalling infrastructure that tightly: a request is received and it shall not be processed if the potential processing result does not match what the client is willing to accept. To me this means that the process needs to stop before even trying to produce the result, because otherwise work will be done that should not have been done. This can be solved by a directive that asserts the ability to respond with a given media type—and I don’t see any way how we could make this magically work without user intervention given that any such approach would need to compute the actual response entity, which is a no-go.
In a perfect and/or weird world we might strive for compile-time verification that all generated responses have matching media type assertions “above” them in the routing structure, but with the currently already very complex magnet pattern type usage I think we have exhausted the complexity budget.
ktoso
commented
8 years ago
Exactly, which is why I honestly don't understand why respondWithMediaType, which does exactly this, is an anti-pattern that is so awful it was not migrated from spray.
ktoso
commented
8 years ago
Nope, this is incorrect: respondWithMediaType transforms the actual response, whereas I am proposing a directive that verifies that the request permits the right media type. These are quite different.
ncanibano
commented
7 years ago +1 This feature will be very helpful for us. It's causing some problems right now. We are migrating from spray to akka-http.
Currently the marshalling infrastructure only gets invoked once the actual response has been produced. This means that content negotiation failures are triggered late, after the backend has already processed the response. For safe methods (e.g. GET) this only results in the wasted work for retrieving the resource (which might already be significant), but for unsafe methods this could actually result in a state change that doesn't get signaled properly to the client.
Currently a marshaller is defined as
which means that switching to a "fail early" model does require modification to the very core of the marshaling infrastructure. However, I currently don't see why it shouldn't be possible.