letting web server commit the response naturally

[nykolaslima]

@Turini @csokol this is related to #1035

[csokol]

Great, @nykolaslima!

But I think your PR broke the unit tests 😞

I guess you will probably need to flush the PrintWriter used on the tests manually.

[nykolaslima]

Yeah! I'm fixing it 😢

[nykolaslima]

@csokol done!
thanks for the tip on custom PrintWriter 👍

[Turini]

thanks @nykolaslima, this PR seems ok to me!

@csokol @lucascs can you guys see any downside by doing this?
if not, i guess we can 🚀 here

[csokol]

I think that removing the early flush should not cause any problems, @Turini. If @lucascs is ok with it, then 🚢 it.

[lucascs]

Imagine that:

I wrote "ok!" to the response. Didn't flush.

Transaction interceptor changes the status code to 500. Flushes.

We'll get a 500 error with ok! on the response.

We're fixing one bug and creating another one.

Maybe a better way to do this is to delay the response actions until after the interceptors run.
So result.use(bla) would simply collect the actions and they would be run after the interceptor stack finishes...

this is a much bigger change, worthy of a major bump in VRaptor version, but we probably should do this eventually

[nykolaslima]

I agree with you @lucascs. But the problem that you related already exists.
For example, in your controller you wrote "ok".
Some error occur after that, let's say the HibernateTransactionInterceptor failed to commit the transaction.
You have another interceptor that catches unexpected exception and wrote "failed because of exception {}".
When the response is flushed, it will containt "okfailed because of exception {}".

I was talking with @csokol to do exactly what you suggested, delay the write to after the interceptors run. But it will required a huge change in the framework.

I think that we are not creating another bug with this PR but we are not solving all bugs (response status and body). What do you think about merging this PR, that solves the response status problem and open a new issue to handle the "delayed response write"?

(ps: to workaround the problem with the messages wrote to the response, the interceptor could inject HttpServletResponse and call reset method -- but it only works if response is not flushed http://docs.oracle.com/javaee/6/api/javax/servlet/ServletResponse.html#reset%28%29)

[lucascs]

👍

[nykolaslima]

Awesome @lucascs. I already opened an issue #1037

[nykolaslima]

@Turini can we merge it?

[Turini]

👌

[Turini]

thanx @nykolaslima

[nykolaslima]

🎉