scrapinghub / extruct

Extract embedded metadata from HTML markup
BSD 3-Clause "New" or "Revised" License
846 stars 113 forks source link

Accept JSON parsing errors in JSON-LD extractor #45

Open giordand opened 7 years ago

giordand commented 7 years ago

When the JsonLdExtractor tries to parse json ld in some web page raise ValueError; no json object could be decoded. My solution was to catch the error in JsonLdExtractor._extract_items(self, node) (because maybe the extractor detected some microdata or rdfa in the webpage but the error only occurs with json-ld, and if we catch the error in extruct.extract we'll lose that data) and by default return an empty list:

def _extract_items(self, node):
        try:
            data = json.loads(node.xpath('string()'))
            if isinstance(data, list):
                return data
            elif isinstance(data, dict):
                return [data]
        except Exception as e:
            print e
        return []
redapple commented 7 years ago

Hi @giordand , thanks for the report. Would you happen to have an example URL where this happens? I have a local stashed change that I had to apply for examples you get from schema.org, the root cause being for me that there was some HTML comment at the start of the script text node

$ git diff HEAD 
diff --git a/extruct/jsonld.py b/extruct/jsonld.py
index fe555db..84dfe74 100644
--- a/extruct/jsonld.py
+++ b/extruct/jsonld.py
@@ -4,6 +4,7 @@ JSON-LD extractor
 """

 import json
+import re

 import lxml.etree
 import lxml.html
@@ -24,7 +25,12 @@ class JsonLdExtractor(object):
                          if item]

     def _extract_items(self, node):
-        data = json.loads(node.xpath('string()'))
+        script = node.xpath('string()')
+        try:
+            data = json.loads(script)
+        except ValueError:
+            # sometimes this is due to some JavaScript comment
+            data = json.loads(re.sub('^(\s*//.*)|(\s*<!--.*-->\s*)', '', script))
         if isinstance(data, list):
             return data
         elif isinstance(data, dict):
giordand commented 7 years ago

Yes, i remember that in my case there was HTML comments too , so it should be fixed when you commit & push that changes. Let me ask you a question , when you commit that changes will it be available with a pip update command to the extruct library?

redapple commented 7 years ago

I'll need to release a new version of extruct for the change to be available directly from PyPI via pip. Note that pip also allows installing from git specific commits

redapple commented 7 years ago

@giordand , it would be most helpful if you can provide a real example of a URL (or the HTML of it) where extruct failed, just to check if my patch really does solve your issue.

giordand commented 7 years ago

@redapple here is the json-ld script wich the jason.loads cannot load:

{
      "@context": "http://schema.org",
      "@type": "Organization",
      "name": "Action Car and Truck Accessories",
      "url": "http://www.actiontrucks.com",
      "sameAs" : [ "https://twitter.com/actioncar_truck",
        "https://www.youtube.com/user/actioncarandtruck",
        https://www.facebook.com/actioncarandtruck],
       "logo": " http://actiontrucks.com/files/images/logo.png",
      "contactPoint" : [
        { "@type" : "ContactPoint",
        "telephone" : "+1-855-560-2233",
        "contactType" : "sales"} ]
    }

Look at the red line, the double cuotes are missing in that element of the array. I did the test completing it with the double cuotes and no error were catched, so here we've got an example where apparently has no solution because the original json object is malformed and surely that object is not loading correctly in the web page. I think that the only solution for this without changing the reality is to catch the error and return an empty list

redapple commented 7 years ago

Thanks for the feedback @giordand . I'd go for catching the parse error, log a warning or error, and return an empty list like you suggest.

vu3jej commented 6 years ago

Observed something similar while working on the same website as in #57; in here

{
"@context":"http://schema.org",
"@type":"Restaurant",
"@id":"https://www.cosaordino.it/locale/906/monza-e-brianza/sedici-piadina",
"name":"SeDICI Piadina",
"image":"https://www.cosaordino.it//pictures/locale/wxthumb/5430632eaa15ffff058debd370253417_thumb.png",
"sameAs":"https://www.cosaordino.it/locale/906/monza-e-brianza/sedici-piadina",
"servesCuisine":"piadine",
"address":{
"@type":"PostalAddress",
"streetAddress":"via Monza, 29",
"addressLocality":"Brugherio",
"postalCode":"20861",
"addressRegion":"Brugherio",
"addressCountry":"IT"
},
"telephone":"039 914 3386",
"geo":{
"@type":"GeoCoordinates",
"latitude":,
"longitude":
},
"aggregateRating":{
"@type":"AggregateRating",
"ratingValue":"0",
"bestRating":"0",
"worstRating":"0",
"ratingCount":"0"
},
"potentialAction":{
"@type":"OrderAction",
"target":{
"actionPlatform":[
"http://schema.org/DesktopWebPlatform",
"http://schema.org/MobileWebPlatform"
],
"inLanguage":"it-IT",
"url":"https://www.cosaordino.it/info/906/monza-e-brianza/sedici-piadina"
},
"deliveryMethod":[
"http://purl.org/goodrelations/v1#DeliveryModeOwnFleet"
]
}
}

Notice the missing double quotes around latitude and longitude values. In this case I fixed it with the following regex, though I doubt there's a catchall solution.

json_str = re.sub(
                pattern=r'(\"\:\s)([^"\{\[])',
                repl=r'":""\2',
                string=json_str
            )
Gallaecio commented 4 years ago

I’m looking at the code, and I see that extract already handles this as suggested depending on the errors parameter, which can be set to log for the suggested behavior, ignore to do nothing or strict (default) to let the exception raise.

When using a specific parser, I think it makes sense to keep the current behavior; users are free to catch the exception of let it raise further.