[wontfix] Usage of libxml_use_internal_errors


#1

Proposal:
disable libxml_use_internal_errors when you’re done with it.

Reason 1:
If we’re handling xml w/o clearing the errors, we might end up using more RAM then necessary.

Reason 2:
I’ve had a really hard time figuring out where some bugs in feediron come from. Someone is not clearing their error queue. :roll_eyes:

Reason 3:
We might discover some bugs. I’m certain we will! Some code pieces blindly clear the queue and other code pieces just set internal handling and ignore all errors (classes/rssutils.php for example).
Since, there’s already an error_handler set this should be without further consequences, except some real bugs popping up.

I’d have sent a pull request, but I wanted to hear your opinions before putting any time into it.

edit:
Also, if a plugin were to disable internal error handling, other pieces of code that rely on it might not receive the needed information.


#2

i assume this is about a plugin (?), why not call libxml_clear_errors() before you start doing things?


#3

If you call libxml_use_internal_errors() it returns the previous state, which you can store and then set it back to when your plugin is done.

Given the prices of VPSs in 2018, I doubt that’s a real concern for an application like TT-RSS. Heck, I managed to get it run on 128 MB when I first started using it.

Maybe, but at the end of the day this particular library is insanely noisy with reporting. Since we’re working with external documents out of our control it’s pretty much guaranteed that the library is going to vomit garbage out all the time.


#4

yeah op i think it’s better if you just assume that errors handled internally is the default
not sure what the big deal is anyway tbh


#5

Yes, this is more or less about a plugin.
Clearing errors would help if I wanted to fix errors that happen inside my code.
But, if I run inside the plugin and happen upon an error that occurred somewhere else, there is no possibility of figuring out where it came from. Except maybe going through all ttrss code, not worth it, IMHO.

Handling errors when they occur is something I quite like. From what I gather, using internal error handling is abused to hide and ignore errors. Which is not something I’m particularly fine with.

If you don’t care about the errors, collect them and throw them away. If you run into serious trouble along the way you’d have a hard time tracking down the source when the errors pile up.

At least that’s my problem:
I get

DOMNode::appendChild(): Couldn’t append node

along with a unhelpful stacktrace. Problem is, when the hook to the plugin is called the error already occurred and I have no way of figuring out where the problem is exactly. Could be a libxml2 bug.
So I’ve recompiled libxml2 (with DEBUG_TREE enabled in tree.c) on so I could get some feedback about the problem.
When the plugin code is called, it seems it’s already too late and the error occurred somewhere earlier. So I’m left with all the uncaught errors from there, with no clue where they actually occurred nor what the problem is exactly.

If the feed’s xml is malformed it should be reported to the owner or user, if the problem lies with ttrss it should be fixed. But I can’t do that if all errors are suppressed.
The only place that does this is feedparser.php.

To summarize

I think that errors should be handled when they matter, otherwise logged or reported to the user. That’s why I brought it up for discussion.


#6

i’m not going to repeat myself just reiterate that

  1. this behavior is not going to be changed
  2. there’s information above on how to deal with your particular problem

#7

fine with me, just wanted to know if I should put in work to fix it or if it isn’t wanted.