Late and safer response writes

[rafanoronha]

Not intended to be merged - just a preview to support further discussion.

This shows how feasible might be my way to address this[1] issue in vraptor core.

Does this piece of code make sense around here?

[1] garcia-jj/vraptor-plugin-hibernate4#18

[lucascs]

I don't know if the request execution is the right place to put this code. Maybe creating another interceptor for this would be better.

as the first interceptor, with the code running after the stack.next(..) call.

[rafanoronha]

I wasn't sure about execution order guarantees, but you are right, probably no one is going to deal with the response before ExecuteMethodInterceptor
Beyond that, do you see this code getting merged?

[lucascs]

The code makes sense, but it could break existing applications...

we should be careful testing with existing applications before shipping this.

And also write some tests for the feature ;)

[garcia-jj]

I can't understand why this is necessary.

[lucascs]

if we write anything to response, it will commit the response, so the status code can't change.

If any error occurs after the result.use(json()).serialize() the user will see a 200 status instead of a 500

It happens, for example, on the transaction interceptor, which commits the transaction after the controller execution.

[rafanoronha]

@garcia-jj serializers write directly to response stream.
Applications using vraptor probably have code that might throw errors after serializers got called. My particular scenario is - usage of vraptor-plugin-hibernate4, the transaction interceptor commits the transaction but any database error might not be reported as an http error. It only gets reported if the response isn't flushed until that moment, and it will be flushed if more data than the response buffer size get written on it.

[rafanoronha]

@lucascs Regarding the shipping of this code, in a first release we might glue it to gson (or any other) serialization. If everything goes fine then we handle other serializers.

[garcia-jj]

Hmm, now I see. I'm worried about performance, because we will store many data into memory as json/xml is too long.

Ideas?

[lucascs]

If we ship we should do it on all serializers.

[rafanoronha]

just realized a second approach for the issue would be deprecating Results methods like json() in favor of interceptors like that one based on @Json. These interceptors should run close to the end of the request pipeline. Also any transaction management interceptor should be able to commit before they get executed.

There is also a third approach - recommending users of the serializers to adjust the response buffer size of the servlet container to a value that fits their response payloads. This can even be made programatically and it actually solves the problem.

[csokol]

I don't know if this is actually possible, but, instead of caching the
content of the response, we could cache only the objects that should be
serialized, and serialize them later in a interceptor, I think this would
solve the memory issue that Otavio mentioned.

Chico Sokol

On Thu, Mar 6, 2014 at 7:01 AM, rafanoronha notifications@github.comwrote:

just realized that a different approach for the issue would be deprecating
Results methods like json() in favor of interceptors like that one based
on @JSON. These interceptors should run close to the end of the request
pipeline. Also any transaction management interceptor should be able to
commit before they get executed.

Reply to this email directly or view it on GitHubhttps://github.com//pull/605#issuecomment-36840611
.

[lucascs]

If we go with the csokol solution, we could add a .commit() method on Serialization instead of creating another component to handle lazy writes.

what do you think?

[rafanoronha]

Yeah sounds like the better approach so far. I'll try to give it a shot