shweshi / OpenGraph

A Laravel package to fetch Open Graph data of a website.
https://opengraph.shashi.dev
MIT License
157 stars 29 forks source link

Log errors from DOMDocument::loadHTML() using libxml_get_errors() #64

Closed shweshi closed 4 years ago

shweshi commented 4 years ago

Retrieve the error by using libxml_get_errors() perform logging or return the list of issues to the caller.

$html = $this->curl_get_contents($url, $lang);
        /**
         * parsing starts here:.
         */
        $doc = new DOMDocument();

        $libxml_previous_state = libxml_use_internal_errors(true);
        $doc->loadHTML('<?xml encoding="utf-8" ?>'.$html);

        //catch possible errors due to empty or malformed HTML
        Log::warning(libxml_get_errors())

        libxml_clear_errors();
        // restore previous state
        libxml_use_internal_errors($libxml_previous_state);

Using Log::warning(libxml_get_errors()) is not compatible with PHP 5.6

alinmiron commented 4 years ago

libxml_get_errors() is available in PHP 5 >= 5.1.0 and PHP 7, according to PHP documentation.

Log::warning is available in illuminate/log package ( 'warning' => MonologLogger::WARNING) way before the version 5.0 of the package; see https://github.com/illuminate/log/blob/5.0/Writer.php ; also, the minimum supported PHP version would be >=5.4.0

we can use in composer.json "illuminate/log": "^5.0|^6.0|^7.0|^8.0", in this way I think that we support many framework versions and of course many PHP versions.

I would like to also use another param on the OpenGraph::fetch method to allow users to enable/disable the logging of the malformed HTML parsing. I'll come with a Pull Request soon

shweshi commented 4 years ago

libxml_get_errors() is available in PHP 5 >= 5.1.0 and PHP 7, according to PHP documentation.

Log::warning is available in illuminate/log package ( 'warning' => MonologLogger::WARNING) way before the version 5.0 of the package; see https://github.com/illuminate/log/blob/5.0/Writer.php ; also, the minimum supported PHP version would be >=5.4.0

we can use in composer.json "illuminate/log": "^5.0|^6.0|^7.0|^8.0", in this way I think that we support many framework versions and of course many PHP versions.

I would like to also use another param on the OpenGraph::fetch method to allow users to enable/disable the logging of the malformed HTML parsing. I'll come with a Pull Request soon

This looks good @alinmiron Thanks.

shweshi commented 4 years ago

Fixed in #65