CFHTTP add a success=true/false to the result (same logic as throwOnError)

language ideas
1

moving the discussion here as jira is not the best place for discussions

LDEV-4175 CFHTTP add a success=true/false to the result (same as throwOnError)

My initial idea was to keep this simple, simply adding a success=true|false which reflects whether an exception would be thrown if throwOnError=true

you can see the underlying java code in question here

Currently, any responses in the 200-299 range trigger an error, or responses that fail without a status code, which @isapir pointed out then populates a status_code of 0, so you end up some boilerplate if you donā€™t want to be throwing a costly exception ( generating stacktraces has a little overhead, depending on how deeply nested your code is etc )

@Brad_Wood and others asked about the logic here and whether 401 responses are successful, they are client errors and need special handling anyway

This is just meant to be an easy way to access the existing cfml business rules to check if the response wasnā€™t typically ok and may need additional handling, without having to wrap the code in a try catch

currently, when throwOnError=false, you need to write the following to match that behavior, this is just about reducing a little boiler plate, itā€™s not mandatory to use this, itā€™s just an option

<h1>throwOnError=true</h1>
<cftry>
    <cfhttp url="http://missing.luce1e.org" result="result" throwOnError=true></cfhttp>
    <cfcatch>
        // check and handle the 0 or 401 etc 

    </cfcatch>
</cftry>    
// just keep on coding, check it's json etc

<h1>throwOnError=false, DIY</h1>
<cfhttp url="http://missing.luce1e.org" result="result"></cfhttp>
<cfif result.status_code gte 200 
        and result.status_code lte 299>
    // definately ok
<cfelseif result.status_code eq 0 >
    // really failed expletives (censored)
<cfelse>
    // may needs extra handling, 401 etc
</cfif>

vs
<h1>throwOnError=false, with result.success</h1>
<cfhttp url="http://missing.luce1e.org" result="result"></cfhttp>
<cfif result.success>
    // definately ok
<cfelseif result.status_code eq 0>
    // really failed expletives (censored)
<cfelse>
    // may needs extra handling, 401 etc
</cfif>

some examples (with and without dump, as that obviously is expensive too!)

erghhh, itā€™s all quite messy infact

2

It seems like busy work to me, with very little merit, and probably actually slightly deleterious.

Considerations:

As it even states in the ticket: what constitutes success/failure is a business-logic decision, not a language-level decision.

Also itā€™s not exactly hard to write an if statement to check the status code, which already provides more on-point information one can work with in the business logic to decide on the next action.

As for it being boilerplate: handling HTTP requests is not really something spends a great deal of time doing, in the bigger context of things. I donā€™t think itā€™s worth messing about with.

Thereā€™s perhaps also a case that you are actually encouraging more boilerplate, as the instinct will be to check success, and then if false one is still likely to need to deal with 3xx, 4xx and 5xx statuses in a bespoke fashion anyhow. One might as well just get on with it and start with the code that deals with the status codes.

I also question the ā€œoverheadā€ of creating a stacktrace when one has just completed an HTTP request (which is intrinsically gonna be a slow operation as it has to go out onto the network). Is anyone actually going ā€œoh man the overhead of generating that stack trace is killing me here!ā€? I doubt it. Sounds to me like the basis for this is premature optimisation.

Plus: it would lead to yet another incompat with CF.

Downvote.

ā€“
Adam

1 Like
3

4xx is an error IMO because when I request coffee and receive a 418, we have an issue.

Joking aside, Iā€™m confused as to what the point is.
Different apps have different needs. Code accordingly.

  • While I didnā€™t realize an invalid host (which really should be a UUID so I am no longer tempted to register luce1e.org) returns a DIFFERENT statuscode from a 404, when using throwonerror=true, one can again, easily write logic to see what was returned.

As for the timing, other than the time to resolve/fail to resolve initial invalid hostname, Iā€™m not seeing much here (a few ms with throwonerror vs not with and without dump). If you create a unique UUID for the invalid hostname, I think youā€™ll see longer times there as well.

My take-away:

throwonerror=true, code in your catch may have something like cfcatch.statuscode?:0

throwonerror=false, code might be checking result.status_code

4

Did you intend to say donā€™t trigger an error?

Cool thatā€™s fine. I was just curious.

Just to throw some ā€œprior artā€ out there. @elpete has a ton of really nice stuff in Hyper (His fluent HTTP library) including separate flags for

  • isError()
  • isClientError()
  • isServerError()
  • isSuccess()
  • isOK()
  • isRedirect()

Overall, Iā€™m find with this enhancement (even tough itā€™s quite simple in comparison to the helper methods Hyper gives us) so long as it receives proper documentation and I would push for Adobe to add it as well. I always prefer to not throw an exception, which only makes it more difficult to get access to the cfhttp results struct.

Thatā€™s a bit miopic. People writing integrations with APIs, which is more and more common, spend a great deal of time handling the results of CFHTTP. Thatā€™s largely what drove Eric to write Hyper. Now, I donā€™t know that adding a single success flag will be any sort of panacea, but itā€™s a decent start.

Yes, actually. I avoid the overhead of throwing unnecessary errors like the plague. Itā€™s known for being rather slow in both CF and in Java when done under load. In fact, a good number of performance bugs Iā€™ve filed with Adobe and Lucee over the years stemmed from some sort of internal exception which was caught that could have just been handled as a flag.

Iā€™ll enter the ticket if no one else will. Even though your statement is correct, it is already the stated intentions of Lucee to be a superset of CFML. Youā€™ve made your displeasure with that decision quite clear over the years, but the fact remains that itā€™s still Luceeā€™s stance, so pointing it out every time is sort of a non-sequitur.

Again, the overhead of exceptions adds up under load. Iā€™ve worked with clients with entire apps written in CFML/Lucee that made 5-10 HTTP calls every request to a Node backend for all their data and just a few ms of extra overhead in Luceeā€™s CFHTTP made their apps rather slow under a lot of traffic.

Iā€™m not sure what this contributes to the discussion. The delay it takes to recognize an bad host is simply part of the networking stack most likely waiting for a DNS lookup. If you have an app which you expect to regularly have invalid hostnames, then this is a delay you canā€™t avoid regardless of whether you throw or not. Iā€™ll also point out itā€™s not a CPU-intensive delay, compared to the overhead of constructing an exception and itā€™s related call stack.

1 Like
5

For me personally throwonerror=true should only throw an error if there is a timeout / connection problem. A 500 response code should be fine and not throw an exception. (I think this is also how OkHttp handles it, (.execute() function)).

If throwonerror=false and a timeout/connection problem occurs, the status code should be set to 0.

Status codes should always be checked/handled by the application.
So a extra success node would not be needed in my opinion.

6

I think you misunderstood what I was attempting to say.
In the linked gist (404s removed to be more-clear): the time resolving the invalid DNS is 100+ms for the first request/example. It has nothing to do with the error handling, itā€™s just how DNS works for the failure.

image
image1505Ɨ475 40.5 KB

This is point is clear with the following: the time it takes with dumps or without and with error handling or without doesnā€™t make any measurable difference for the discussion

image
image1663Ɨ469 42.8 KB

7

we arenā€™t changing any existing behaviour, this is just optionally adding a flag which bypasses the need to throw and expose the existing business rules, if you want to use it

8

So if I understand correctly, the proposal is
I see a new key in the result success=true|false representing: a non-2xx status code saying
Hey, just letting you know had you passed throwonerror=ā€œtrueā€ an error would have been raised

1 Like
9

I believe it does, and I explained above how delay waiting for a network DNS call does not bog a server the same as a CPU intensive operation of creating an exception. That aside, saying ā€œI found some other slow stuff in my code, so that means we can ignore this optimizationā€ seems a little odd. Especially since that use case of an invalid host name seems like more of an edge case.

10

Keep in mind when I talk about performance, Iā€™m talking about a heavily loaded server, not your local machine.

Go and run 50 threads for a minute against some code and then youā€™ll see what I mean.

2 Likes
11

I love to talk about performance. One reason I like Lucee is the commitment to performance.

I get that the time it takes to generate a stack is less about the actual clock time and more about the energy/CPU to do it (especially if you throw that steak away).

Currently: I donā€™t fully understand what the proposed change is.
I do understand the why: performance.

I donā€™t understand how itā€™s intended to be used. I set another flag besides throw on error so I can bypass the exception and still know about it?
Or it just does it when I donā€™t set throw on error? Iā€™m not sure, the ticket didnā€™t make it clear, I missed the discussion detail.

I trust others to make an informed decision but a little better user story, test case, acceptance criteria, or dumb it down for this dog / others to get me on board.

12

What I understand is that there are usecases where you would just want a ā€œsuccessā€ return from cfhttp, instead of deep diving into error struct details programatically.

Sincerely, I needed such a flag back in 2007. At that time we implemented a function where users would submit an URL and we would check that URL for some content (as an e.g. it could be a simple meta-tag verification code).

At that time we would have liked just to get a simple result with ā€œsuccessā€ back from cfhttp, so we would contiune to verify the content of the result to find the meta tag and parse it.

Tons of different cfhttp errors would be thrown: network errors, DNS errors, redirects, failing redirects (to a non existing location URL, 403 errors, charset(!!!) erros because of badly configured web servers, SSL handshake errors, just to note a bunch of possible errors. And many times you wonā€™t even get an http status code back to get the 200 status and move on, because many errors can be thrown at an earlier point. Because of performance, we also started pre-fetching data using the method attrbute with value ā€œHEADā€, but many servers just donā€™t support the head method (some admins block that method from their servers).

Oh yes, I would have needed such a success flag back then, and having it as an option is great. If you donā€™t want it, then donā€™t use it. If you want to dig into error details for business logic, you can dig into the error struct and do conditional stuff. But there are times you would just want ā€œsuccessā€ back and go on with checking the file content.

I like the option and you have my vote. But use ā€œsuccessā€ and not ā€œokā€, because that is also widespread in the Javascript world (many ajax tools use the term ā€œsuccessā€).

1 Like
13

Thanks @andreas ā€¦ when I read ā€œflagā€ I think of an attribute /arg Iā€™m passing with the http call.

What new arg, that wasnā€™t clear.

It was obvious there would be a change to the response (which is optional to use).

So it sounds like the proposed change is only passing back another key in the response?

1 Like
14

huh??? I thought the title was pretty self explanatory?

CFHTTP add a success=true/false to the result (same as throwOnError)

maybe this is slightly clearer

CFHTTP add a success=true/false to the result (same logic as throwOnError)

15

@Phillyun I was gently starting to think in that direction too, but then I saw the Jira ticket linked in @Zackster s post, and that made it clear.

16

Got it. :upside_down_face:
I originally read the title as meaning:

Proposed: If I do not want an error raised pass the new flag: (for performance)

http url="foo.bar" skipThrowOnError=true

Current: If I do want an error raised:

http url="foo.bar" throwonerror=true

Either will be returning the success=true/false

17

why???

18

I didnā€™t see the point of having to pass something in to get something else out either!

  • I think this proposed change seems appropriate and harm free.

This discussion has raised a question I hesitated to ask until the scope/intent was clear (for me).
Others believe per spec, other status codes are also successful.
How difficult would it be to get this success logic exposed?

Iā€™m flexible on format if documented.

this.tag.cfhttp.successStatusCodes = "200-299,410,418"
19

To date, I never have been to a restaurant that only has one thing on the menu and I never have seen a programming language that only has ā€œhello worldā€.

More options are better.

2 Likes
20

POC

2 Likes