CFLint icon indicating copy to clipboard operation
CFLint copied to clipboard

Suggestion:: Report <cflog> tags with @type=Error that are outside of a cfcatch statement.

Open ryaneberly opened this issue 6 years ago • 8 comments

ryaneberly avatar Oct 03 '17 20:10 ryaneberly

what's wrong with logging an error outside a catch?

zspitzer avatar Oct 03 '17 23:10 zspitzer

Without knowing @ryaneberly's exact scenario, I'd agree that it's in general probably not good practice. If you log an error, that error should generally be handled in some way.

TheRealAgentK avatar Oct 04 '17 00:10 TheRealAgentK

Sorry for putting an issue out here without much context. We were cleaning up our log server internally, and I was observing that most of the 'false positive' errors were coming from cflog statements that were incorrectly given the type='error'. And most of our valid cflog/@type=error are inside the cfcatch blocks --- not all of them though.

The few other valid cflog/@type=error examples I saw that were not inside cfcatch were standalone statements in cfelse blocks.

This experience may not match the rest of the world though. What are your thoughts?

ryaneberly avatar Oct 04 '17 11:10 ryaneberly

Not all errors are caught, for example you have to inspect the result of cfhttp, which may return a result you want to log as an error

zspitzer avatar Oct 04 '17 23:10 zspitzer

@zspitzer that is actually a valid scenario of using cflog @type=error outside try/catch, yes.

TheRealAgentK avatar Oct 04 '17 23:10 TheRealAgentK

Let's keep in mind though that it'd be only one of many rules. And we have ways to deal with rules re individually don't agree on and therefore don't want to use.

So, if it was to become a rule, individuals could be manually ignore certain usage. Or people could just exclude the whole rule.

TheRealAgentK avatar Oct 04 '17 23:10 TheRealAgentK

Could it be opt in, instead of opt out?

zspitzer avatar Oct 04 '17 23:10 zspitzer

I think rule groups is a good mechanism for opt-in.

ryaneberly avatar Oct 05 '17 15:10 ryaneberly