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.

Sign up for GitHub

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

Jump to bottom

Error in jsonld parse #24

Open
floflock opened this issue Sep 14, 2018 · 12 comments · May be fixed by #25
Open

Error in jsonld parse #24

floflock opened this issue Sep 14, 2018 · 12 comments · May be fixed by #25

Comments

@floflock
Copy link

Hey there,

how can I prevent this error thrown by jsonld-parser.js?
The input html code has no jsonld, this why w-a-e is throwing the error.

Maybe, w-a-e should try if json-ld is present, because the error message is wrong at the moment if now json-ld is in the html - right?

Error in jsonld parse - SyntaxError: Unexpected end of JSON input                                                                 
POST /detailpage 200 4729.179 ms - 678
POST /detailpage 200 5661.727 ms - 2388
Error in jsonld parse - SyntaxError: Unexpected end of JSON input
POST /detailpage 200 4616.781 ms - 664
Error in jsonld parse - SyntaxError: Unexpected end of JSON input                                                                 POST /detailpage 200 4659.136 ms - 1339
@paambaati
Copy link

@floflock Can you include a code snippet to reproduce this issue?

@floflock
Copy link
Author

floflock commented Sep 14, 2018 •
edited

Sure, here is the code:

const wae = require('web-auto-extractor').default

const html = '<html></html>' // from a request of my development server

wae().parse(html)

Maybe you try this page:
https://www.docmorris.de/melissaphosphorus-compmischung/01632860

See data here:
https://search.google.com/structured-data/testing-tool/u/0/#url=https%3A%2F%2Fwww.docmorris.de%2Fmelissaphosphorus-compmischung%2F01632860

@paambaati
Copy link

@floflock This should be easy to handle by wrapping the .parse() call with a try/catch.

@floflock
Copy link
Author

ok, that's clear for me, but I thought it was conceptional wrong to throw syntax error, if there is no json-ld present. :)

@paambaati
Copy link

Throwing is better, so you know your input isn't in the expected format.

@floflock
Copy link
Author

Try/catch isn't possible, because it is a `console.log()``

try {
      return wae().parse(html)
} catch (e) {
      //
}

@paambaati
Copy link

@floflock Okay so the error isn't thrown, it is just a log statement; it should be ignorable.

@TheDahv
Copy link

TheDahv commented Dec 17, 2018

FWIW I'm working on this now and expected it to throw. I do get the log message, but that's not as explicit as throwing and handling a specific case.

Alternative designs:

  • explicitly declare this operation throws -- that should feel normal to JS developers since anybody who has ever used JSON.parse has had to consider that case (whether they knew it or not)
  • populate an errors property on the instance I can inspect manually

TheDahv added a commit to TheDahv/web-auto-extractor that referenced this issue Mar 7, 2019
@TheDahv
Store JSON-LD parse errors
2fb2e22 
Web Auto Extractor logs and skips parse errors it encounters when
working on JSON+LD. However, a program cannot react to messages written
to console.

This change allows a developer to hook into parse errors and react to
them if desired.

Fix indix#24
@TheDahv TheDahv linked a pull request Mar 7, 2019 that will close this issue
Open
@TheDahv
Copy link

TheDahv commented Apr 11, 2019

Hi @paambaati, I trust that you're all quite busy so I don't mean to come off with any undue urgency.
I'm curious to know what you would advise regarding #25

Is Indix open to contributions on this project? Given your estimation on time available to review submissions, do you recommend I work off a fork for a while?

@floflock
Copy link
Author

floflock commented May 4, 2019

@paambaati would you be so kind to give short feedback on that topic?
As far as I can see, @paambaati wants to add additional features.
I will prepare the package for exnext-ready syntax (Class, etc.)...

@paambaati
Copy link

@TheDahv @floflock First off, apologies for the silence; we've been busy for the past few months. To answer your questions, yes, please go ahead with your fork. I'll carve out some time to review it.

@floflock
Copy link
Author

floflock commented May 6, 2019

@paambaati thanks for your feedback. I use your package in a certain project. I've made a fork and will try to improve some things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Store JSON-LD parse errors TheDahv/web-auto-extractor
3 participants
@TheDahv @floflock @paambaati