Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept JSON parsing errors in JSON-LD extractor #45

Open
giordand opened this issue Jun 11, 2017 · 8 comments
Open

Accept JSON parsing errors in JSON-LD extractor #45

giordand opened this issue Jun 11, 2017 · 8 comments

Comments

@giordand
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

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.

@redapple redapple changed the title Error parsing Json-Ld in JsonLdExtractor Accept JSON parsing errors in JSON-LD extractor Jun 13, 2017
@vu3jej
Copy link

vu3jej commented Nov 11, 2017

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
Copy link
Member

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.

bhavya17037 added a commit to bhavya17037/extruct that referenced this issue May 29, 2020
Add jsonStringFixer.py, which has a function to add quotes around any required
text in a json string. Used this in jsonld.py to handle invalid jsonld string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants