Request Timeout (was:Lucee and the future)

[Michael Offner]

Inside the "Request Timeout" thread was a discussion going on that was pointing out that a request timeout cannot be catched by a try/catch, what seems possible in ACF.

In my opinion this is a bug in ACF here here is my explanation why:

"Like i said, a request timeout is like pulling the plug, i assume you do not pull the plug on your computer when you are done working, this has the risk to destroy open file when they are in a transition. You only pull the plug when the computer is frozen.

With running requests it is exactly the same, a request timeout always comes with the risk to destroy something, maybe the request was just writing a file that then is only halfway done or you have open streams that not get closed properly anymore ...
so you should avoid request timeouts at any coast and never ever try to deal with them.
if a request timeout occurs, you need to find out why and solve that problem.

that is btw exactly the same as caching runtimeexcpetions in java, that smells!"

The reason i'm writing all of this is because something else come to my mind we discussed some time ago, sometimes it is maybe impossible to "fix" a code segment that leads to a request timeout. Simply because that code segment, for example, is dealing with external resources, you have no influence on how long it takes for them to get done.

I had that problem in the Lucee admin when calling the ExtensionProvider to get information from them. So i did the following (simplified):

// call the extension provider (maybe takes some time)

thread name="getData" {

    request.data=getDataFromExtensionProvider();

}

// give the thread a chance to finish

end=getTickCount()*10000;// now + 10 seconds

do {

   sleep(100);// take a nap 

}while(getData.status!="completed" && getTickcount()>end); // wait until the thread is done or we waited more than 10 seconds

if(getData.status!="completed") return request.data;

throw "not able to get data";

So the idea is to do a new tag to does exactly this:

timeout timespan="#createTimeSpan(0,0,0,10)#" result="res" {

   data=getDataFromExtensionProvider();

}

if(res.status=="completed") return data;

throw "not able to get data";

So you have a code block where you can define how long Lucee maximal can spend time on it and stopping only that block if you want.

So you can avoid a request timeout by using this tag for time sensitive code fragments.

What do you think?

Micha

[Adrian Lynch]

Sounds OK.

Feels like it's a bit of a bandaid to fix a bigger issue though.

If my request timeout is set to 60 seconds and I have to look at 6 external sources, I have to do the maths on how long each has, plus the main requests likely time. 60 / 10, +/- a second. I can't do that kind of maths!

Does onRequestEnd run on a request timeout? Could there be a way to pass something to onRequestTimeout from the new proposed timeout tag? That way the cleanup code stays with the initial code but the responsibility for running in is moved to the request life cycle.

Just thinking out loud.

Adrian

--
You received this message because you are subscribed to the Google Groups "Lucee" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lucee+un...@googlegroups.com.
To post to this group, send email to lu...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEBwyM%3DddD%3DbCr5WhzO0ngGDc7e-McoNAsB0ag8s8iv69DQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Dominic Watson]

I like the general idea of arbitrary timeouts. Not sure on the syntax though. How about something along these lines?

timeout timespan=mytimeout {

 a bunch of code

} ontimeout  {

 // stuff to do when the timeout is hit

} oncomplete {

 // stuff to do when complete w/ no timeout

}

or:

executeWithin( timespan, executionClosure, timeoutExceptionHandlingClosure );

On 12 February 2015 at 09:13, Michael Offner <mic...@lucee.org> wrote:

--
You received this message because you are subscribed to the Google Groups "Lucee" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lucee+un...@googlegroups.com.
To post to this group, send email to lu...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEBwyM%3DddD%3DbCr5WhzO0ngGDc7e-McoNAsB0ag8s8iv69DQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

Pixl8 Interactive, 3 Tun Yard, Peardon Street, London SW8 3HT, United Kingdom T: +44 [0] 845 260 0726• W: www.pixl8.co.uk• E: in...@pixl8.co.uk

Follow us on: Facebook Twitter LinkedIn

CONFIDENTIAL AND PRIVILEGED - This e-mail and any attachment is intended solely for the addressee, is strictly confidential and may also be subject to legal, professional or other privilege or may be protected by work product immunity or other legal rules. If you are not the addressee please do not read, print, re-transmit, store or act in reliance on it or any attachments. Instead, please email it back to the sender and then immediately permanently delete it. Pixl8 Interactive Ltd Registered in England. Registered number: 04336501. Registered office: 8 Spur Road, Cosham, Portsmouth, Hampshire, PO6 3EB

[Michael Offner]

In my opinion the request timeout is a bandaid, because when a request timeout is happening it is already to late.

Your math is the wrong way around, you should define how long a certain code block needs to execute, not the entire request. When you do a estimate for work that need to be done, you do not first define how long it takes overall and than break this to pieces, no you do it the other way around, the calculated time for the single pices gives you the complete estimate.

This timeout tag has nothing to do with the request timeout, it does not end the, so there is no need to pass something to a specific event handler.

Micha

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAH368SRjD%2B_BHLvEFAB8qEbjEg-EH-PSO0NxmsnYkBZA1GzdbQ%40mail.gmail.com.

[Michael Offner]

I was thinking about closures as event handler, but in my opinion there is simply no need, this is for the user happening in the same thread and you end in any case after the tag.

From the flow it is very similar to cflock.

Micha

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAEYvUxmtmDv1SNFFJjjnfGduXZciPrGfe4aKQHRr8jvm-1akAQ%40mail.gmail.com.

[Dominic Watson]

Sure, I kinda like it as an alternative, but not overly fussed.

However, for a new 'tag', I think having a return value specified as a variable in an attribute should be avoided as discussed in plenty of places. I'd personally prefer a try catch style syntax if sticking with tags:

timeout timespan=timeoutValue {

 // your timeout protected code here

} exired {

 // your cleanup code here

}

I think that this, or something like it, is cleaner than:

timeout timespan=timeoutValue result="timeoutResult" {
 // your timeout protected code here
}

if ( !timeoutResult.success ) {
 // your cleanup code here
}

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEBy_rVfTYOnh4%3DbX6A_txCu7KtwV8p6XBxU5b4T0NxxqjQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Dominic Watson]

I really like the idea of a micro timeout functionality btw.

Also, interestingly, I think I'm right in saying that Open Bluedragon does not have a general request timeout. By default at least, I believe it will allow a request to run as long as it likes. Alan's position being that it isn't up to the engine to decide.

I kind of like this approach, especially when married with your suggestion.

[Adam Cameron]

On Thursday, 12 February 2015 23:23:35 UTC+13, Dominic Watson wrote:

executeWithin( timespan, executionClosure, timeoutExceptionHandlingClosure );

This. Not the version Micha suggested, which was goddawful. Any syntax suggestion like Micha's is going to result in me thinking of this: http://blog.adamcameron.me/2015/01/cfml-evolution-of-functionality.html

That said, there's two different things afoot here.

Secondly, there's this "code timeout" idea which you're discussing the solution for above. I think you've nailed the syntax above, Dom.

Firstly there's recovery from out-of-control requests. I don't think the current solutions are correct, but unlike Alan BlueDragon, I do think it's the job of the system to pay attention to. And unlike Railo/Lucee, I think it's the application's job to possibly tidy-up after.

I've not given this much thought, but I was wondering about an additional handler in the application framework (in Application.cfc, but might need to be in Server.cfc?):

public boolean function onTimeoutReached(context);

Returning true from this would let the request continue, at least for another timeout period, before firing onTimeoutReached() again. Returning false would end the request (onRequestEnd() fires). Raising an exception in this would cause onError() to fire.

I dunno what the context argument would contain, but something that would allow the handler to possibly deal with whatever is going on at the time the timeout was reached.

Thoughts?

-- 

Adam

[Adam Cameron]

On Thursday, 12 February 2015 23:23:35 UTC+13, Dominic Watson wrote:

I like the general idea of arbitrary timeouts. Not sure on the syntax though. How about something along these lines?

timeout timespan=mytimeout {

 a bunch of code

} ontimeout  {

 // stuff to do when the timeout is hit

} oncomplete {

 // stuff to do when complete w/ no timeout

}

Oh... meant to point out that this syntax doesn't need the oncomplete block. The code to run "oncomplete" is simply the next line of code after this statement. 

Still: your closure example syntax is better.

-- 

Adam

[Michael Offner]

between the lines ...

Micha

On Fri, Feb 13, 2015 at 2:32 AM, Adam Cameron <dac...@gmail.com> wrote:

On Thursday, 12 February 2015 23:23:35 UTC+13, Dominic Watson wrote:

executeWithin( timespan, executionClosure, timeoutExceptionHandlingClosure );

This. Not the version Micha suggested, which was goddawful. Any syntax suggestion like Micha's is going to result in me thinking of this: http://blog.adamcameron.me/2015/01/cfml-evolution-of-functionality.html

thanks for the inspiring words ;-)

this is consistent with the current syntax (see cflock), i always try to see the biiger picture and try to be pragmatic in my solutions.

so let's look at my example again:

timeout timespan="#createTimeSpan(0,0,0,10)#" result="res" {

   data=getDataFromExtensionProvider();

}

and this is how cflocks works today (what is very similar from the purpose, that is the reason i have used it as a template)

lock timeout="#createTimeSpan(0,0,0,10)#" name="whatever" {

   data=getDataFromExtensionProvider();

}

like you can see this is consistent with the existing syntax, my example was not about the syntax, it was about the idea for the tag timeout ....

i could also use the ACF syntax instead

timeout(timespan="#createTimeSpan(0,0,0,10)#", result="res") {

   data=getDataFromExtensionProvider();

}

But that makes no difference. We discussed once that we could add the possibility that script tags can return values, so that means you could do something like this

res=timeout(timespan="#createTimeSpan(0,0,0,10)#") {

   data=getDataFromExtensionProvider();

}

i would like to merge script tags with BIFs in the future, but that is an other discussion and i try to not mix things up...

In my opionion adding a closure in that case is simply a overhead ...

 

That said, there's two different things afoot here.

Secondly, there's this "code timeout" idea which you're discussing the solution for above. I think you've nailed the syntax above, Dom.

this syntax forces me to have my executed code in a separated scope ... 

Firstly there's recovery from out-of-control requests. I don't think the current solutions are correct, but unlike Alan BlueDragon, I do think it's the job of the system to pay attention to. And unlike Railo/Lucee, I think it's the application's job to possibly tidy-up after.

that is my point, a request timeout has nothing to do with "tidy-up after"

it is "hell something is very wrong here, you need to do something! Please let that never ever happen again! Please!"

When you have a request timeout in your log you have to solve it, because it can harm your system! You dont deal with it in your code!

When you have rats in your house you try to get rid of them, not make a arragement with them!

 

I've not given this much thought, but I was wondering about an additional handler in the application framework (in Application.cfc, but might need to be in Server.cfc?):

public boolean function onTimeoutReached(context);

this way we embrace people to let request timeout happen and accept them as part of a normal program flow. what is the complete wrong message! No rats in my house.

 

Returning true from this would let the request continue, at least for another timeout period, before firing onTimeoutReached() again. Returning false would end the request (onRequestEnd() fires). Raising an exception in this would cause onError() to fire.

 

I dunno what the context argument would contain, but something that would allow the handler to possibly deal with whatever is going on at the time the timeout was reached.

it is to late to deal with it, you maybe have open streams to whatever (files, datasources, external services ...) and you have no clue about it. There are millions of possibilities that can cause a request timeout!

Let's say the request was just writing a file, not only that this file is only half way done, even worse the stream to this file is still open and maybe blocked (under windows for sure!) for any other request until the system is restarted.

the request timeout cannot clean up, it is a hammer!

 

Thoughts?

-- 

Adam

--
You received this message because you are subscribed to the Google Groups "Lucee" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lucee+un...@googlegroups.com.
To post to this group, send email to lu...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/6c80ab26-435f-4a9a-9de1-47722060c2b0%40googlegroups.com.

[Dominic Watson]

I'm getting mixed messages Micha. "Consistent with the existing syntax". "We want to create a new syntax to move away from the baggage of the old syntax". Another option, that would also be consistent with existing syntax:

try {

    timeout timespan=myTimeout throwontimeout=true {

        // code to execute within myTimeout

    }

} catch( timeout e ){}

@Adam, love the idea of an onRequestTimeout application lifecycle event that might allow some kind of recovery.

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEByB6HsDcxKHz88FYOtXbv1Mu6X7CBEeiSCBRD-3ct5tww%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Dominic Watson]

@Micha, for me, I don't like the idea of having a return value from the timeout - especially not in having to define the variable that gets the result in an attribute.

What would be in that returned value? Just success / failure? If so, either throwing an error or having some try / catch style syntax would remain consistent and give a cleaner syntax I think.

I would also argue that for consistency, both a tag and a function could/should be implemented. For the function with closure, you would rightly need to ensure that other functionality across the board was using a similar syntax and that you had nailed the roadmap for that so as to keep consistency. And FWIW, the function name executeWithin() is not clear enough, perhaps executeWithinTimeout( timeout=485, code=closure ).

[Michael Offner]

That is not about .lucee, that was about a new feature, remember what ever happen, new feature are not affected by the dialect.

still any kind of "recovery" from a request timeout is sending the wrong message.

Micha

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAEYvUx%3DnhW5F62_rcnZHSacOD%3D1C1NH_-zc%3D0-UtKrx%3DXbW08w%40mail.gmail.com.

[Dominic Watson]

Gotcha @micha. I think my main gripe though is that if you are to build new features, use the best bits of the existing syntax and not continue with the odd ones (i.e. declaring return values in attributes, especially when that return value may not really be needed).

Re sending the wrong message on timeouts - programmers can set the request timeout per request, why should they not then be able to decide what to do when that limit has been reached *before* the execution is interrupted? Indeed, if onError() is already invoked on request timeout, could you not just as easily invoke a more specific onRequestTimeout()?

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEBxe%3D1R%3DD3wYngaOyzQZLoRncAOp7yMLvB6h2bSwTKZ61w%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Michael Offner]

between the lines ...

Micha

On Fri, Feb 13, 2015 at 1:27 PM, Dominic Watson <dominic...@pixl8.co.uk> wrote:

@Micha, for me, I don't like the idea of having a return value from the timeout

like i said that is a different topic and not about "timeout", it is about that script tags can be handled as expressions.

 

- especially not in having to define the variable that gets the result in an attribute.

you still have to decide waht you are doing with the return value ... like with a function call as well...

What would be in that returned value? Just success / failure?

what makes sense...

 

If so, either throwing an error or having some try / catch style syntax would remain consistent and give a cleaner syntax I think.

when i think about, this is similar to try/catch, so i could even think about the following

try(timeout:createTimeSpan(0,0,0,1)) {

}

catch(timeout e){

   dump("shit happens");

}

when i think about i like that a lot!

it's a little bit more complicated to implement (needs some parser ajustments), but it is worth doing it!

 

I would also argue that for consistency, both a tag and a function could/should be implemented. For the function with closure, you would rightly need to ensure that other functionality across the board was using a similar syntax and that you had nailed the roadmap for that so as to keep consistency. And FWIW, the function name executeWithin() is not clear enough, perhaps executeWithinTimeout( timeout=485, code=closure ).

executeWithinTimeout is to long

 

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAEYvUxkP2D3srM_3NjkdW8YMYswO%3DR8BaFm1PzxE3q1g58b-_w%40mail.gmail.com.

[Dominic Watson]

try( timeout=CreateTimeSpan( 0, 0, 0, 1 ) ) {

}

catch(timeout e){

   dump("shit happens");

}

Is very interesting indeed! Not entirely sure about it (seems a little like making one thing do two things). But definitely interesting. My initial thought was a new construct similar to try catch:

executeWithinTimeout timespan {

 // code to do stuff with

} timeout( t ){

 // error handling

}

I wonder if there are any other language examples to steal from? ... goes to look on the interwebs....

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEBzPnXsZgv4kB66ANKeyUSy4U5PjiwsHBb0aH5FUgAFsrw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Dominic Watson]

Ruby stdlib timeout: http://ruby-doc.org/stdlib-1.9.3/libdoc/timeout/rdoc/Timeout.html

Just throws an error on the timeout (or returns the value of the codeblock - which would not make sense in the cfml tag context, though it would in the function with closures context).

[Michael Offner]

between the lines ...

Micha

On Fri, Feb 13, 2015 at 1:57 PM, Dominic Watson <dominic...@pixl8.co.uk> wrote:

Gotcha @micha. I think my main gripe though is that if you are to build new features, use the best bits of the existing syntax and not continue with the odd ones (i.e. declaring return values in attributes, especially when that return value may not really be needed).

counter argument is "consistent", what is very important for me. when i add a new features i always look for existing pattern to do so.

 

Re sending the wrong message on timeouts - programmers can set the request timeout per request, why should they not then be able to decide what to do when that limit has been reached *before* the execution is interrupted? Indeed, if onError() is already invoked on request timeout, could you not just as easily invoke a more specific onRequestTimeout()?

i'm not happy with the current implementation at all, i'm with alan here, we only support this for compatibility reasons to acf.

Like i said i see the in the same light as catching nullpointerexceptions in java, it is exactly the same reason you should not do it.

see this:

http://stackoverflow.com/questions/18265940/should-i-always-not-catch-nullpointerexception

this says it all:

"it is next to impossible to know whether it is safe to attempt to recover, and / or to know what "remediation" might be required. So the best thing to do is to let the NPE propagate to the base level, and then treat it as a generic bug. " 

just replace "NPE" with "request timeout"

wi will do a new section "good practice" in the wiki that will include this

 

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAEYvUxk4HQgeg15MRk8wTwZf4rXTuF1sTbSBSCvQvGs0pbAZvg%40mail.gmail.com.

[Michael Offner]

java has extended the try statement to close "closable" object automatically, so doing "2 jobs" with try catch is not uncommon.

http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

Micha

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAEYvUx%3DsyR4qCAhTmXjX9-YcKqMHrSNqfyMXo4wri7CbiCk4Zg%40mail.gmail.com.

[Dominic Watson]

You can remain consistent without reusing the bits that you don't like (fwiw, I'm not against using tags, just returning a variable this way, especially when not needed). Of course, if you like the returnvar="someVariable" syntax, that's fine ;)

Whether or not you allow the user to recover from the timeout or not, having a precise event for cleaning up after timeouts maybe useful to someone. i.e. doesn't allow you to recover from some unknown problem, but does allow you to identify it sharply.

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAG%2BEEBwHuspuMKbqVj%3DsMtvD%3DdcH1BJZt02VN26GyZ462mdBEw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Jesse Shaffer]

I do like the various suggestions for a timeout block - probably prefer the block syntax (specifically the try syntax) over a closure syntax - and the block syntax would be particularly useful in AOP situations.

component {
    public function invokeMethod(invocation) {
        var meta = getMetaData(invocation.getTarget());

        param name="meta.timeout" default=10;
       
        try (within=meta.timeout) {
            return invocation.proceed();
        } catch (TimeoutException e) {
            // handle the timeout
        }
    }
}

As far as the event handler - I see both sides of the issue.  Yes - if there is a timeout, the request has exceeded its bounds and needs to be shut down.  How long should the handler be allowed to prolong the request after its supposed timeout?  Do you give the timeout handler another timeout?

However...for the application/server as a whole, being able to automatically take action when requests start timing out would be nice.  It can allow applications to attempt "self-healing", meaning fewer support calls, better customer satisfaction, greater developer interest, world peace... etc.

What about allowing the event handler in the Application.cfc in addition to the timeout block, as long as it is fired in the background, therefore not blocking the errant request from completing?  I think that could be a decent compromise.

[Dominic Watson]

> try (within=meta.timeout) {
>    return invocation.proceed();
> } catch (TimeoutException e) {
>  // handle the timeout
> }



^^^^ +1 for 'within'. That reads really naturally.


[Igal @ Lucee.org]

I like this construct, but instead of "within" I would use "timeout", which is consistent with cflock.

Igal Sapir
Lucee Core Developer
Lucee.org

--
You received this message because you are subscribed to the Google Groups "Lucee" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lucee+un...@googlegroups.com.
To post to this group, send email to lu...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/CAEYvUxm6OgaYw4K4cpu88n9VOUQYhybRoUAD6aWODySnYL%2B0BA%40mail.gmail.com.

[Adam Cameron]

On Friday, 13 February 2015 22:55:56 UTC+13, Micha wrote:

This. Not the version Micha suggested, which was goddawful. Any syntax suggestion like Micha's is going to result in me thinking of this: http://blog.adamcameron.me/2015/01/cfml-evolution-of-functionality.html

thanks for the inspiring words ;-)

You already knew my opinion of your efforts with this syntax.

 

this is consistent with the current syntax (see cflock), i always try to see the biiger picture and try to be pragmatic in my solutions.

It is consistent with the current syntax, this is true. I guess my position is that the current syntax should be obsoleted ASAP and replaced with a better approach. Accordingly, I don't want to perpetuate that syntax by adding even more instances of it. However you have a point in that in the interim, it's perhaps best to not add in yet another approach to skinning the same at.

I'd take each case on merit though. Perhaps where there's completely new functionality at work, I'd look to not perpetuate the earlier bad approach if something can simply be implemented as a function / method instead.

 

res=timeout(timespan="#createTimeSpan(0,0,0,10)#") {

   data=getDataFromExtensionProvider();

}

This is a better intermediary step. If you want to stick with this block approach, at the very least you need to stop the current approach of setting the return value via an attribute/string combination.

 

i would like to merge script tags with BIFs in the future, but that is an other discussion and i try to not mix things up...

In my opionion adding a closure in that case is simply a overhead ...

Overhead doesn't matter. Language should be about clarity for the developer, and predictability of the code. I think the horse has bolted as far as what you've done to CFML, but there's still a chance not to mess up .lucee. (TBH, I am only considering new features to be targeted at .lucee, so I am not necessarily considering existing CFML syntax when I make suggestions).

If you need to do work to improve the speed of function execution: so be it. If you need to be slightly more clever in what you compile it to? So be it. Language constructs should never be about how convenient it is for you to compile and execute.

Also there simply shouldn't be a meaningful overhead when it comes to executing a function! It's such a core language operation you can't be shying away from that as an approach simply because of how you compile and run the code. If there's an issue: make it faster.

 

Firstly there's recovery from out-of-control requests. I don't think the current solutions are correct, but unlike Alan BlueDragon, I do think it's the job of the system to pay attention to. And unlike Railo/Lucee, I think it's the application's job to possibly tidy-up after.

that is my point, a request timeout has nothing to do with "tidy-up after"

it is "hell something is very wrong here, you need to do something! Please let that never ever happen again! Please!"

When you have a request timeout in your log you have to solve it, because it can harm your system! You dont deal with it in your code!

My position is that if the dev can control the duration of the time out (they can), then they should also be able to control what happens when the timeout is reached. Or at least have the option to.

 

When you have rats in your house you try to get rid of them, not make a arragement with them!

I wish you'd dispense with the vapid metaphors.

 

public boolean function onTimeoutReached(context);

this way we embrace people to let request timeout happen and accept them as part of a normal program flow. what is the complete wrong message! No rats in my house.

You sound like Rupesh (from the ColdFusion Team) now. I don't think it's for you to dictate to me how I write my application, sunshine. It's also a false analogy anyhow because all we're suggesting is to provide a hook for the odd occasion when the solution for a request timeout being reached might be something to deal with differently from just ignoring it (Alan BlueDragon), or generically erroring out (CF).

 

I dunno what the context argument would contain, but something that would allow the handler to possibly deal with whatever is going on at the time the timeout was reached.

it is to late to deal with it, you maybe have open streams to whatever (files, datasources, external services ...) 

Yeah, this was my thinking too. I just know there's some event handlers / interceptors in Lucee that don't provide any context for why they fired, and it makes them pretty much useless.

Even if it's a line number and operation which was running at the time, and the info was "read only", that'd be something.

-- 

Adam

[Adam Cameron]

On Saturday, 14 February 2015 01:20:49 UTC+13, Dominic Watson wrote:

I'm getting mixed messages Micha. "Consistent with the existing syntax". "We want to create a new syntax to move away from the baggage of the old syntax".

Yeah. Dom, are you like me in that for all these new features, you're only considering them or .lucee, not CFML?

Perhaps threads need to be more clear about that. Or Lucee needs to be more clear about how they are considering where new features will be homed.

@Adam, love the idea of an onRequestTimeout application lifecycle event that might allow some kind of recovery.

I think Micha's right in that the options for recovery might be minimal. But at least having the option to ignore the timeout or raise a specific exception, or log something specific would be nice. And not beyond the realms of what is reasonable in the given situation.

-- 

Adam

[Adam Cameron]

On Saturday, 14 February 2015 01:49:02 UTC+13, Micha wrote:

That is not about .lucee, that was about a new feature, remember what ever happen, new feature are not affected by the dialect.

What does this mean?

 

still any kind of "recovery" from a request timeout is sending the wrong message.

Just remember that whilst this is your opinion on the matter, there are opinions which are at odds with this. 

-- 

Adam

[ADK]

Or 2 group's: 1 for Lucee-CFML (nee Railo) and one for Lucee-New. Most of the angst so far has been due to this confusion.

[Adam Cameron]

On Saturday, 14 February 2015 02:17:16 UTC+13, Micha wrote:

java has extended the try statement to close "closable" object automatically, so doing "2 jobs" with try catch is not uncommon.

http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

ONE language doing it still makes it "uncommon" ;-)

I guess you mean "not without precedent". 

That Java thing is interesting though: didn't know about it.

-- 

Adam

[Adam Cameron]

On Friday, 13 February 2015 14:32:32 UTC+13, Adam Cameron wrote:

On Thursday, 12 February 2015 23:23:35 UTC+13, Dominic Watson wrote:

executeWithin( timespan, executionClosure, timeoutExceptionHandlingClosure );

Having surveyed all the suggestions made, and kept an open mind about them, I still think this is the most natural, clean approach.

One downside Micha had about this is the performance of closures. I think this is intrinsically moot here, given we're already talking about long-running code!

I also think the more code we facilitate being run in isolated memory blocks the better, and code that has been identified to possibly run into timeout issues is also a sitter for being isolated like this too.

I have stopped to think about whether it clashes with that existing block syntax I dislike, but I am not sure it does. It's just a function... we're already got functions. We've also already got closure functions too, so it's more really just exploring and extending the opportunities closures give the language, rather than conflicting with syntax. I don't see as there being a conflict.

-- 

Adam

 

[Adam Cameron]

On 14 February 2015 at 09:11, ADK <and...@leftbower.com> wrote:

Or 2 group's: 1 for Lucee-CFML (nee Railo) and one for Lucee-New. Most of the angst so far has been due to this confusion.

Well I think most of the confusion was unfounded, but yes, either way it existed. This is a good suggestion to solve it!

-- 

Adam

[Jesse Shaffer]

On Friday, February 13, 2015 at 2:19:58 PM UTC-6, Adam Cameron wrote:

On Friday, 13 February 2015 14:32:32 UTC+13, Adam Cameron wrote:

On Thursday, 12 February 2015 23:23:35 UTC+13, Dominic Watson wrote:

executeWithin( timespan, executionClosure, timeoutExceptionHandlingClosure );

Having surveyed all the suggestions made, and kept an open mind about them, I still think this is the most natural, clean approach.

From a functional perspective, the closure syntax makes sense - especially if you're constantly switching between cfscript and javascript.  However, comparing the two in the AOP example I gave: 

function invokeMethod(invocation) {
   try (within=30) { // or timeout

      return invocation.proceed();
   } catch (TimeoutException e) {
      // handle the timeout
   }

   // vs

   // now an extra variable is introduced - potentially wasting memory?
   var result = javacast("null","");
   executeWithin(createTimespan(0,0,0,30), function() {
      result = invocation.proceed();
   }, function() {
      // handle the timeout
   });
   
   return result;
}

The closure syntax feels way too verbose and doesn't feel like an actual feature of the language.  It feels like an afterthought.  It also deals with the timeout as an event, instead of an exception, which is what it should be IMO.

The try syntax has the added benefit of timeouts as only one type of exception:

try (within=30) { // or timeout
   var fileToProcess = fileRead("/path/to/resource");
   // process file into the database...
} catch(TimeoutException e) {
   // just took way too long...
} catch(MissingFile e) { // or whatever the exception should be in case fileOpen fails...
   // file isnt there
} catch(Database e) {
   // error inserting records
} finally {
   // easy to tell if this gets called if the timeout is reached
}

// vs

executeWithin(createTimeSpan(0,0,0,30), function() {

   try {
      var fileToProcess = fileRead("/path/to/resource");
      // process file into the database...
   } catch(MissingFile e) { // or whatever the exception should be in case fileOpen fails...
      // file isnt there
   } catch(Database e) {
      // error inserting records
   } finally {
      // what about now? does this block execute if execution exceeds the timeout?
   }

,function() {
   // took way too long...
   // handling the timeout here feels unnecessarily disjointed
});
 

The try syntax is, to me, the best overall syntax for dealing with these types of timeouts.

[Christian Meis]

+1 for the try variant. Looks natural to me, and as far as I'm concerned would be the best fit. Other variants discussed look much too noisy to me.

Regarding the general question whether there should be a chance to handle a timeout in the code:
For me that would be a logical extension for exception handling. "Normally" timeouts shouldn't occur and probably mostly are effects of a bug or bad code. Then we should better fix our code than trying to repair the effects. I can imagine situations though (e.g. connections to external services /resources) where timeouts might occur and could be "expexted".
I don't see why we shouldn't get the chance to react here and go on gracefully in certain cases.

Christian

[Igal @ Lucee.org]

which one? there were a few of them...

[Walter Seethaler]

Hi Micha,

at first a big thank you that you reraised the topic for open discussion, I really appreciate that. In ACF it was comfortable to simply catch a request timeout, when the "designed" request runtime was blown up to send notifications or update the import log in the db or something else. But I trust your assessment here, because you have clearly more insight about what happens internally on a request timeout. Even more, the proposed solution is even better than catching a request timeout, because you have full control over the timeout in your code block. 

IF we consider avoiding request timeouts a principle, I guess it should be recommended in the docs (and explained) and marked as a ACF incompatiblity, eg.

so you should avoid request timeouts at any coast and never ever try to deal with them.
if a request timeout occurs, you need to find out why and solve that problem.

Not sure, are request timeouts automatically captured somewhere or do I need to log them in the onError?

Adam mentioned that the server should clean up resources automatically, does Lucee do that eg. open files on a request timeout. 

New syntax! I think try(timeout... is a weak language design, because where you control the timeout and where you want to catch it, is not always the same place - it is similar to the lock:

try{
    someLockInHere();
    someTimeoutInHere();
}catch(Lock e){

}catch(Timeout e){
}

I would stick with the lock syntax but have found no word as good as "lock" yet. Even some attributes may fit:

block
   timeout=150
   throwOnTimeout=true    (default - there may be cases where you dont want to throw on timeout)
{
}

Could even imagine to integrate attributes for locking, but probably we should KISS.

On the technical side.

I remember on ACF where some DB calls didn't even trigger the request timeout. Would the implementation of that feature really be that robust to interrupt everything on timeout and raise an exception? If thats not possible in every case, it would be good if it is documented well.

Cleaning up resources? If I do a fileOpen() and the timeout occurs, would Lucee close the file for me or would I have to do it in the catch?

Again, thank you very much for discussing that topic.

Cheers

Walter

PS: I owe you at least a beer at cfcamp:)

Am Donnerstag, 12. Februar 2015 10:13:31 UTC+1 schrieb Micha:

Inside the "Request Timeout" thread was a discussion going on that was pointing out that a request timeout cannot be catched by a try/catch, what seems possible in ACF.

In my opinion this is a bug in ACF here here is my explanation why:

"Like i said, a request timeout is like pulling the plug, i assume you do not pull the plug on your computer when you are done working, this has the risk to destroy open file when they are in a transition. You only pull the plug when the computer is frozen.

With running requests it is exactly the same, a request timeout always comes with the risk to destroy something, maybe the request was just writing a file that then is only halfway done or you have open streams that not get closed properly anymore ...
so you should avoid request timeouts at any coast and never ever try to deal with them.
if a request timeout occurs, you need to find out why and solve that problem.

that is btw exactly the same as caching runtimeexcpetions in java, that smells!"

The reason i'm writing all of this is because something else come to my mind we discussed some time ago, sometimes it is maybe impossible to "fix" a code segment that leads to a request timeout. Simply because that code segment, for example, is dealing with external resources, you have no influence on how long it takes for them to get done.

I had that problem in the Lucee admin when calling the ExtensionProvider to get information from them. So i did the following (simplified):

// call the extension provider (maybe takes some time)

thread name="getData" {

    request.data=getDataFromExtensionProvider();

}

// give the thread a chance to finish

end=getTickCount()*10000;// now + 10 seconds

do {

   sleep(100);// take a nap 

}while(getData.status!="completed" && getTickcount()>end); // wait until the thread is done or we waited more than 10 seconds

if(getData.status!="completed") return request.data;

throw "not able to get data";

So the idea is to do a new tag to does exactly this:

timeout timespan="#createTimeSpan(0,0,0,10)#" result="res" {

   data=getDataFromExtensionProvider();

}

if(res.status=="completed") return data;

throw "not able to get data";

So you have a code block where you can define how long Lucee maximal can spend time on it and stopping only that block if you want.

So you can avoid a request timeout by using this tag for time sensitive code fragments.

What do you think?

Micha

[Christian Meis]

Sorry - posting from my mobile clearly had some disadvantages as to losing oversight.

I meant this variant:

try (within=30) { // or timeout
var fileToProcess = fileRead("/path/to/resource");
// process file into the database...
} catch(TimeoutException e) {
// just took way too long...
} catch(MissingFile e) { // or whatever the exception should be in case fileOpen fails...
// file isnt there
} catch(Database e) {
// error inserting records
} finally {
// easy to tell if this gets called if the timeout is reached
}

Even proper quoting seems impossible on Google groups mobile view...

Christian

[Michael Offner]

Please see my answers between the lines.

Micha

On Saturday, February 14, 2015, Walter Seethaler <seet...@nbsp.com> wrote:

Hi Micha,

at first a big thank you that you reraised the topic for open discussion, I really appreciate that. In ACF it was comfortable to simply catch a request timeout, when the "designed" request runtime was blown up to send notifications or update the import log in the db or something else. But I trust your assessment here, because you have clearly more insight about what happens internally on a request timeout. Even more, the proposed solution is even better than catching a request timeout, because you have full control over the timeout in your code block. 

IF we consider avoiding request timeouts a principle, I guess it should be recommended in the docs (and explained) and marked as a ACF incompatiblity, eg.

so you should avoid request timeouts at any coast and never ever try to deal with them.
if a request timeout occurs, you need to find out why and solve that problem.

Not sure, are request timeouts automatically captured somewhere or do I need to log them in the onError?

They have their own log file, I think the name is request-timeout.log 

Adam mentioned that the server should clean up resources automatically, does Lucee do that eg. open files on a request timeout. 

 There is no way to do that, we speak about any non atomar process that is not directly bound to the request, the possibilities are endless here, even for files it is impossible, there is no registration for file streams, so how should I know that there is a open stream that should not be open.

New syntax! I think try(timeout... is a weak language design, because where you control the timeout and where you want to catch it, is not always the same place - it is similar to the lock:

try{
    someLockInHere();
    someTimeoutInHere();
}catch(Lock e){

}catch(Timeout e){
}

I would stick with the lock syntax but have found no word as good as "lock" yet. Even some attributes may fit:

block
   timeout=150
   throwOnTimeout=true    (default - there may be cases where you dont want to throw on timeout)
{
}

Could even imagine to integrate attributes for locking, but probably we should KISS. 

On the technical side.

I remember on ACF where some DB calls didn't even trigger the request timeout. Would the implementation of that feature really be that robust to interrupt everything on timeout and raise an exception? If thats not possible in every case, it would be good if it is documented well.

Lucee trigger this in any case, ACF has graceful timeouts what makes them partially useless. 

Cleaning up resources? If I do a fileOpen() and the timeout occurs, would Lucee close the file for me or would I have to do it in the catch?

Request timeout is pulling the plug, this is done by a other thread that has no clue what the request does, that is the reason you should avoid request timeouts at any coast. 

--
You received this message because you are subscribed to the Google Groups "Lucee" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lucee+un...@googlegroups.com.
To post to this group, send email to lu...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/edee0d70-0070-4e4c-ae45-cdf9b9b5ed72%40googlegroups.com.

[Adam Cameron]

On Saturday, 14 February 2015 10:01:32 UTC+13, Jesse Shaffer wrote:

From a functional perspective, the closure syntax makes sense - especially if you're constantly switching between cfscript and javascript.  However, comparing the two in the AOP example I gave: 

function invokeMethod(invocation) {
   // [...]

   // now an extra variable is introduced - potentially wasting memory?
   var result = javacast("null","");
   executeWithin(createTimespan(0,0,0,30), function() {
      result = invocation.proceed();
   }, function() {
      // handle the timeout
   });
   
   return result;
}

Well that's easily solved if you simply don't write so much code:

function invokeMethod(invocation) {
   return executeWithin(createTimespan(0,0,0,30), function() {
      return invocation.proceed();

   }, function() {
      // handle the timeout
   });
}

The variation here is that executeWithin() can easily be implemented to return something if that was something that would be useful. It makes a kinda sense, and allows you do be more expedient with your requirement.

One could either specify the event handler for the time out, or if just omitted, then a time out exception is raised, to be handled elsewhere:

function invokeMethod(invocation) {
   return executeWithin(createTimespan(0,0,0,30), function() {
      return invocation.proceed();
   });
}

You only need to deal with it differently if there's a need to. 

These two variations are more descriptive of what's actually being coded for, I think.

 

The closure syntax feels way too verbose and doesn't feel like an actual feature of the language.

Odd. Functions taking "closures" already is part of the language. a try/catch with a time out baked into it isn't.

 

 It feels like an afterthought.  It also deals with the time out as an event, instead of an exception, which is what it should be IMO.

I disagree there. The time out occurring definitely is an event, and this entire discussion is about how to handle that event. That being the case, using an event handler is kinda the natural approach, surely?

I do agree that generally a time out situation is an exceptional circumstance, but clearly we're discussing a situation where that's not automatically the solution. So offering a non-exception-throwing option by implementing an event handler for it... is actually what's being asked for here!

TBH I think your example here is making my case, not your case. You are baking the time out into the try statement, whereas you point out it's not actually relevant to most of the possible exceptional circumstances in that block. That says "poor implementation" to me.

I'd've written your code like this:

try {
   executeWithin(createTimeSpan(0,0,0,30), function() {

      var fileToProcess = fileRead("/path/to/resource");
      // process file into the database...

   }); // note I am not handling the time out here: just letting it error-out

} catch(Timeout e) {
   // just took way too long...

} catch(MissingFile e) { // or whatever the exception should be in case fileOpen fails...
   // file isnt there
} catch(Database e) {
   // error inserting records
} finally {

   // whatever
}

If you've already got code which handles exceptions, just use 'em.

Or if the time out handler wasn't optional (however it definitely should be), just do this in place of the executeWithin() call in the code above:

   executeWithin(createTimeSpan(0,0,0,30), function() {

      var fileToProcess = fileRead("/path/to/resource");
      // process file into the database...

   }, function(){

      throw(type="Timeout");

   });

Again, in this situation use the error handling you've already got there.  And it separates the concerns of "dealing with code that might run too long", and "handling exceptions". Which are two different things.

On the other hand, is the exception handling for the missing file / DB error supposed to be part of the code that might time out? It could be! In which case, obviously that does belong in the executeWithin() call too. Not in exception handling which is excluded from the time out.

Still: you're entitled to your preferred approach here. I'm not suggesting you're not. Nor am I suggesting you're intrinsically wrong. However I'm also not finding myself thinking "hey, Jesse's got a point here".

Good discussion though, and it's encouraged me to better evaluate the viability / suitability of the function-based solution. I like it more now ;-)

-- 

Adam

[Jesse Shaffer]

Adam, thanks for taking the time to make very good points.  It has made me reconsider my position, and while I am not necessarily against executeWithin, I am still like the try syntax a little more, or even a different option altogether.  Let me explain:
 

Well that's easily solved if you simply don't write so much code:

function invokeMethod(invocation) {
   return executeWithin(createTimespan(0,0,0,30), function() {
      return invocation.proceed();
   }, function() {
      // handle the timeout
   });

The variation here is that executeWithin() can easily be implemented to return something if that was something that would be useful. It makes a kinda sense, and allows you do be more expedient with your requirement.

Side note, I would say with executeWithin(), it would be a must for it to return the value of whichever callback returns first.

One could either specify the event handler for the time out, or if just omitted, then a time out exception is raised, to be handled elsewhere:

function invokeMethod(invocation) {
   return executeWithin(createTimespan(0,0,0,30), function() {
      return invocation.proceed();
   });
}

You only need to deal with it differently if there's a need to. 

These two variations are more descriptive of what's actually being coded for, I think.

Except, the only variation I would expect to see would be the first - more on that below...

 

The closure syntax feels way too verbose and doesn't feel like an actual feature of the language.

Odd. Functions taking "closures" already is part of the language. a try/catch with a time out baked into it isn't.

Agreed - I wrote that poorly, and I'm not sure if I can convey my thoughts on it correctly - however I'll try.  I still feel the closure syntax feels too verbose.  What I intended to point out was that dealing with timeouts as a concept feels less integrated with the language itself.   Sure, it is a built-in function, integrated with the language, but honestly (and expounding on this would be its own thread), when I have the option, I think I would prefer syntax constructs or objects as opposed to procedural functions, especially when I'm dealing with otherwise object-oriented code. 

 It feels like an afterthought.  It also deals with the time out as an event, instead of an exception, which is what it should be IMO.

I disagree there. The time out occurring definitely is an event, and this entire discussion is about how to handle that event. That being the case, using an event handler is kinda the natural approach, surely?

It's only an event because you're introducing it as one with executeWithin.  Without calling that method - if the code causes a timeout, the request times out and is an exception, correct?
 

I do agree that generally a time out situation is an exceptional circumstance, but clearly we're discussing a situation where that's not automatically the solution. So offering a non-exception-throwing option by implementing an event handler for it... is actually what's being asked for here!

The highlighted part is what I really want to zero in on, down below...

Expanding on my last question, here's what I see:
1) It's an exception if the code times out outside of executeWithin(), in the form of an overall request timeout (which apparently cannot be handled gracefully?)
2) It's an exception if the code times out inside of executeWithin(), if a handler is not specified, which can be handled gracefully.
3) It's an event if there is a handler specified.

So in one instance, timeouts are being treated differently.  I think that's what I mean by timeouts should always be dealt with as exceptions.  As far as events go - I see exceptions as a specialized subset of events.  They are things that happen, for which you place code that properly handles them.

Again, in this situation use the error handling you've already got there.  And it separates the concerns of "dealing with code that might run too long", and "handling exceptions". Which are two different things.
 

The separation of concerns is perhaps the best counter-argument.

My gut feeling on this is that if you are going to go to the trouble of wrapping a long running block of code, you should be ready to handle it at that point, and not just automatically pass along a different flavor of exception.  For that reason, I would probably lean toward the timeout handler being required.  If you intend to not handle the timeout then you should throw the exception.  At that point, both ways (closures/trys) are essentially equivalent, only the try syntax is faster for me to read and understand what is going on.  Nor do I think its a stretch for someone reading it for the first time to understand it quickly.

However, as always, I can think of alternate scenarios, where you do intend to simply throw an exception.  That brings this new thought to mind.  Lets say you have a component method that should throw an exception if it takes to long to run.  You could either wrap the body in an executeWithin, from which you throw an exception explicitly, or... put the timeout on the function:

function mayTakeAWhile() timeout=30 {} // reads very succinct

try {
   mayTakeAWhile();
} catch(Timeout e) {
   ...
}

// or if you're doing something inline you can use try/timeout

// if timeout is implemented on the function def, then this should follow suit, although I do think try/within is easier on the eyes
try (within/timeout:30) {
} catch(Timeout e) {}

 

On the other hand, is the exception handling for the missing file / DB error supposed to be part of the code that might time out? It could be! In which case, obviously that does belong in the executeWithin() call too. Not in exception handling which is excluded from the time out.

My point was I saw a possibility of a race condition where a database exception occurs at the same time the timeout occurs, in which case, are the catch/finally blocks allowed to complete?  From the way the request timeout currently operates, I'm not sure that it would.  If they are allowed to complete - could the timeout handler execute first? Possibly.  Are these major blockers - no, just extra things to consider.  However, these are non-issues with the try/catch.

try (within/timeout:30) {

   // try doing this task in 30 seconds
} catch (Timeout e) {
   // handle the timeout specifically
} catch (<whatever> e) {
   // handle a different exception here
} finally {
   // will always execute even if a timeout
}

If the code you're writing should have the timeout separate from the db/file exceptions:

try {
   try (within/timeout:30) {

      ...
   } catch (timeout e) {...}
} catch(...) {...}

By the way, I would expect this to work also:

try (within/timeout:30) {

} catch (any e) {
    // handles timeout exceptions here also
}

Another side note - if you have timeouts integrated in with the try/catch (or function), you can get more pinpoint location of where the timeout occurred - page/line/etc - for you to potentially identify the root cause faster.  With executeWithin, all you know is something within the first callback took too long.  Maybe you know what it is, but maybe not.

 

Still: you're entitled to your preferred approach here. I'm not suggesting you're not. Nor am I suggesting you're intrinsically wrong. However I'm also not finding myself thinking "hey, Jesse's got a point here".

I'm wondering if the real Adam Cameron actually wrote that. ;) Perhaps my point is to make sure anything that gets done is fully thought out first - win-win :)
 

Good discussion though

Agreed :)  You are highly respected in the community, myself included, and it has been fun hashing this out.
 

, and it's encouraged me to better evaluate the viability / suitability of the function-based solution. I like it more now ;-)

I've been told I'm stubborn...I still like the try/catch better, and now I like the function timeout idea better too. :)

Jesse

[Dominic Watson]

+1 for try syntax. Seems the cleanest and most succinct (more so than both my previous suggestions). I find try( within... more natural to read than try( timeout ... but not averse to either.

--
You received this message because you are subscribed to the Google Groups "Lucee" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lucee+un...@googlegroups.com.
To post to this group, send email to lu...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/lucee/54DE6FDE.5020506%40lucee.org.

For more options, visit https://groups.google.com/d/optout.

[Adam Cameron]

I'm going to be brief here and only address specific points. Where I've truncated stuff I either agree, don't sufficiently disagree to comment, or our opinions are sufficiently divergent it doesn't warrant further discussion.

On 16 February 2015 at 03:25, Jesse Shaffer <dajest...@gmail.com> wrote:

Side note, I would say with executeWithin(), it would be a must for it to return the value of whichever callback returns first.

Yup, that was my intent.

 

   return executeWithin(createTimespan(0,0,0,30), function() {

One thing that occurred to me over night is that createTimeSpan() is the wrong mechanism to use here. It should just be a unit of seconds (poss ms). 

 

Agreed - I wrote that poorly, and I'm not sure if I can convey my thoughts on it correctly - however I'll try.  I still feel the closure syntax feels too verbose.  What I intended to point out was that dealing with timeouts as a concept feels less integrated with the language itself.   Sure, it is a built-in function, integrated with the language, but honestly (and expounding on this would be its own thread), when I have the option, I think I would prefer syntax constructs or objects as opposed to procedural functions, especially when I'm dealing with otherwise object-oriented code. 

I get what you mean, and in general I err towards OO code over procedural where possible too. However I also think cocking around adding new language constructs is something that should be treated with extreme caution.

My challenge here is that I think adding a time out to try/catch is just wrong, so another approach is needed. It's not that I think it's better or worse than a closure-based option (or even the other syntax variation mentioned, which I'll get to), I just think it's absolutely the wrong and a syntactically invalid approach.

Some people would fall back to that "tags without angle brackets in script" syntax for a solution, which - IMO - isn't as bad in theory, however I think the actual syntax variation is lazy and inelegant language design which ought to be phased out of CFML... in favour of a "closure" approach.

I dunno if you read my blog or saw my earlier link, but I put my position forward here: http://blog.adamcameron.me/2015/01/cfml-evolution-of-functionality.html

In summary if we were talking about purely tag-based code, we could have this:

<cfexecutewithin timeout="1">

    <!--- etc --->

</cfexecutewithin>

That's fine.

Now in Railo/Lucee's CFML, the script-based equivalent to that is currently:

executewithin timeout="1" {

    // etc

}

As per my article above, that approach is goddawful, IMO.

The natural evolution of the thought process from tags to script ought to have continued along the lines I blogged, and resulted in:

executeWithin(1, callback);

Not just for this specific example, but for any block-based functionality.

That's how I got to where I did.

I need to not fall into the same trap Micha did with his generic approach though: is a generic approach actually the right approach all the time? No, it won't be. And I think this is what's being explored by you and others now. I've still not seen anything that compels me to think the non-generic approach gives us anything. And as I said, the time out on a try/catch is a show-stopper for me.

But you have a point re the procedural-ish-ness here.

I can think two things:

* like it or not, most of native CFML is still procedural. It's been getting better since Railo 4.0, but it's still miles behind what you or I would do with our own CFML-written code. The question is: should we perpetuate this? And is messing with existing language constructs or adding new ones so as to not perpetuate more procedural headless functions the correct approach?

* decide that this is a method of a class. System.executeWithin()? Request.executeWithin()? Perhaps it's just a static method of one of Lucee's own classes? I'd like to go that way, TBH: ditch all procedural functions in favour of methods on classes / objects. It does mean actually creating some of the "missing" classes though. Is it OK to have the System (for example) class implied, so headless functions are implicitly methods of that class?

These are more questions than answers.

Also bear in mind that I also see all enhancements as only applying to .lucee code, so we can be ambitious as to how the implementation could be. If .lucee went ahead, I would expect all the headless procedural functions to go. Well: I'll be requesting it.

1) [...] an overall request timeout (which apparently cannot be handled gracefully?)

I think Micha is wrong on that, but that's another thread.

 

I'm omitting the rest as we simply don't agree on this. Which is cool.

 

My gut feeling on this is that if you are going to go to the trouble of wrapping a long running block of code, you should be ready to handle it at that point, and not just automatically pass along a different flavor of exception.  For that reason, I would probably lean toward the timeout handler being required.

Overnight I came back to that way of thinking too. However In the course of thinking about all this again this morning (all of this is pre-coffee too!), I've swung back.

I think there is value-add in just being able to time-box the code block, whether or not the handling of the time out automatically errors, or requires a handler callback, or might take an optional one.

I also think it's legit for the request to exception-out if the time-boxed code exceeds its allowed execution time. I think that generally just failing the request with a Timeout exception is going to be the remedial action in these situations? And having an alternative handling of it (which may then end with an exception of some sort after doing some other stuff) would itself be the exceptional handling of the situation?

 

  If you intend to not handle the timeout then you should throw the exception.  At that point, both ways (closures/trys) are essentially equivalent, only the try syntax is faster for me to read and understand what is going on.  Nor do I think its a stretch for someone reading it for the first time to understand it quickly.

I do not dispute that try/catch syntax is easier to read for the first time. However you hit the nail on the head. This is only ever a problem for the first time one encounters it. After that you do understand it. And, TBH, is not using inline callbacks pretty much second nature for anyone who uses JS these days (and don't all CFMLers use JS too?) I realise understand the minutiae of how closure words eludes a lot of people, but I don't think inline functions are really that foreign for people?

All new syntax is new to a person once.

 

However, as always, I can think of alternate scenarios, where you do intend to simply throw an exception.  That brings this new thought to mind.  Lets say you have a component method that should throw an exception if it takes to long to run.  You could either wrap the body in an executeWithin, from which you throw an exception explicitly, or... put the timeout on the function:

function mayTakeAWhile() timeout=30 {} // reads very succinct

try {
   mayTakeAWhile();
} catch(Timeout e) {
   ...
}

// or if you're doing something inline you can use try/timeout

// if timeout is implemented on the function def, then this should follow suit, although I do think try/within is easier on the eyes
try (within/timeout:30) {
} catch(Timeout e) {}

It's not the job of a function to know whether it might take too long to run in whatever context it's being called in. This is a compile time conceit here, which isn't the right approach.

You're also making the function do two things: a) it's job; b) and timeout if needs must. Functions should only do one thing. The subtle difference here from the executeWithin() function is its one job is to time-box a callback.

 

 

On the other hand, is the exception handling for the missing file / DB error supposed to be part of the code that might time out? It could be! In which case, obviously that does belong in the executeWithin() call too. Not in exception handling which is excluded from the time out.

My point was I saw a possibility of a race condition where a database exception occurs at the same time the timeout occurs, in which case, are the catch/finally blocks allowed to complete?  From the way the request timeout currently operates, I'm not sure that it would.  If they are allowed to complete - could the timeout handler execute first? Possibly.  Are these major blockers - no, just extra things to consider.  However, these are non-issues with the try/catch.

Don't follow you here sorry. Try again? (I might just be being thick).

 

Another side note - if you have timeouts integrated in with the try/catch (or function), you can get more pinpoint location of where the timeout occurred - page/line/etc - for you to potentially identify the root cause faster.  With executeWithin, all you know is something within the first callback took too long.  Maybe you know what it is, but maybe not.

You're inventing a shortcoming here. Remember my suggestion was to pass some context to the time out handler to accommodate this very thing. Even if I hadn't suggested it, you're presupposing a shortfall in something that hasn't been implemented yet simply to further your case ;-)

 

Still: you're entitled to your preferred approach here. I'm not suggesting you're not. Nor am I suggesting you're intrinsically wrong. However I'm also not finding myself thinking "hey, Jesse's got a point here".

I'm wondering if the real Adam Cameron actually wrote that. ;) Perhaps my point is to make sure anything that gets done is fully thought out first - win-win :)
 

It's always the real Adam Cameron.

I treat with respect people who treat both themselves and myself with respect (whichever level of respect is warranted, which in the case of the latter is sometimes "not much" ;-)

I treat with contempt people who insult either their own intelligence, or try to do same with mine.

Good discussion though

Agreed :)  You are highly respected in the community, myself included, and it has been fun hashing this out.

 

, and it's encouraged me to better evaluate the viability / suitability of the function-based solution. I like it more now ;-)

I've been told I'm stubborn...I still like the try/catch better, and now I like the function timeout idea better too. :)

I suspect we are going to ultimately agree to disagree with this one. But let's see where it goes.

You've certainly got me irked about the whole "procedural function" thing today. And I'm not sure my response was "explanation" (good), or "self justification" (bad). I think it's mostly the former, but I'm not sure.

And now I need to go find some coffee.

Cheers fella.

-- 

Adam

 

[Adam Cameron]

On 16 February 2015 at 08:31, Adam Cameron <dac...@gmail.com> wrote:

I'm going to be brief here 

It's the lies I can't stand.

-- 

Adam 

[Jesse Shaffer]

I get what you mean, and in general I err towards OO code over procedural where possible too. However I also think cocking around adding new language constructs is something that should be treated with extreme caution.

Yup.

 

My challenge here is that I think adding a time out to try/catch is just wrong, so another approach is needed. It's not that I think it's better or worse than a closure-based option (or even the other syntax variation mentioned, which I'll get to), I just think it's absolutely the wrong and a syntactically invalid approach.

This is where we must agree to disagree.  It's certainly unprecedented, but that in and of itself is not a reason to not avoid it.  It could be I'm missing something, but I do see a certain elegance in it.  It reads well to me.

Some people would fall back to that "tags without angle brackets in script" syntax for a solution, which - IMO - isn't as bad in theory, however I think the actual syntax variation is lazy and inelegant language design which ought to be phased out of CFML... in favour of a "closure" approach.

I dunno if you read my blog or saw my earlier link, but I put my position forward here: http://blog.adamcameron.me/2015/01/cfml-evolution-of-functionality.html

I have read it, and since you asked, TBH I'm not entirely sold that its bad form.  I think all that script tags need is to return its result instead of assigning it to a variable based on a required attribute.   I think I remember that being suggested somewhere by Micha or Igal.  I agree that ACF devs completely blew it with their cftag() {} syntax, especially after partially implementing the Railo/Lucee style.

...

My point was I saw a possibility of a race condition where a database exception occurs at the same time the timeout occurs, in which case, are the catch/finally blocks allowed to complete?  From the way the request timeout currently operates, I'm not sure that it would.  If they are allowed to complete - could the timeout handler execute first? Possibly.  Are these major blockers - no, just extra things to consider.  However, these are non-issues with the try/catch.

Don't follow you here sorry. Try again? (I might just be being thick).

I was trying to think of potential problems with implementation.  Say you have a try inside the first callback.  The outside thread times out the callback execution at the same time a database exception is triggered.  What gets run? Does the catch block execute in addition to the timeout handler?  In what order?  Does the callback execute in a separate thread? Is it always the same order?  I have spent too much of my life chasing down race condition errors, I'm perhaps hypersensitive to it.

 

 You're inventing a shortcoming here. ...

What can I say, you caught me.  Not knowing what could potentially be put into the context argument, I cannot argue what it cannot do.

Well, that's about all I got, at least for now.  Sounds like several threads may need to be started from this convo, but it's way too late for that right now, at least for me.

Jesse 

[Desmond Miles]

Hi all,

Are there any updates or common workaround about this thread and the issue ?

I use Lucee 4.5.1 and I need to catch requestTimeout exceptions. I'm not focusing on the syntax but more on the feature needed.

I don't want to "pull out the plug" but only notified when a requestTimeout occurs and perform some processing (e.g in another thread limited by the same request timeout) no matter if the timed out thread continues .

[Desmond Miles]

After browsing Lucee's sources I noticed a difference between my current version and the master branch.

In my current version (4.5.1.000), the RequestTimeoutException is not put into the PageContext and a try/catch's "finally" block is always executed even if there's a request timeout.

That way I could do some processing in the "finally" block before the request timeout is reported in logs. But I cannot get the RequestTimeoutException.

Event I can find a workaround to thrown my own exception by checking processing duration, I didn't insist...

In the master branch the RequestTimeoutException is added to the PageContext when the request timeout is detected.

Then the PageContext's thread is properly stopped (instead of deprecated Thread.stop()) so "finally" block is no longer executed as before.

The RequestTimeoutException is still available through PageContext.getStopPosition() and that was not the case in 4.5.1.000

This behaviour is not perfect but it allows me to workaround and meet my needs (e.g checking getStopPosition() in onRequestEnd() for REST component and/or onError() for else).

So I've just updated from 4.5.1.000 to 4.5.1.015 (I could have manually picked an higher version but this was the only version Lucee server admin suggest me)

I think it's not a good practice because I rely on Lucee's internal implementation but I need to be notified on request timeout errors...

Most notably I've seen that the page context is released before the RequestTimeoutException is created and passed to SystemUtil.stop().

Thus I not really sure if I can rely on PageContext.getStopPosition() but it is my only choice here...

I've tried it out and decide later if I come back to 4.5.1.000 and its "finally" block (not better).

[Desmond Miles]

Sadly Application.onError() and onRequestEnd() are NOT called when a CFTHREAD is request timeout (w/o a join with the current request thread).