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

Store JSON-LD parse errors #25

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

TheDahv
Copy link

@TheDahv TheDahv commented Mar 7, 2019

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 #24

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

TheDahv commented Mar 7, 2019

Interesting note about the failing tests: Travis runs tests against node v5.12. I wrote this code against v10.12.

It appears Node has different error messages:

  1) Web Auto Extractor when there are parse errors should save jsonld parse errors:
      AssertionError: expected [ Array(2) ] to deeply equal [ Array(2) ]
      + expected - actual
       [
      -  "Unexpected end of input"
      -  "Unexpected token '"
      +  "Unexpected end of JSON input"
      +  "Unexpected token ' in JSON at position 11"
       ]

I have a couple ideas, and I'm interested in your take:

  • write code that checks for fuzzy/near-matches of error messages, or matches against a set of known error messages
  • adjust the inner library to wrap parse errors and expose them with a consistent message
  • don't check for message type at all but just count reported errors to prove things are working correctly

Node produces different error messages for JSON parse errors depending
on the node.js version (Travis is pinned on an old Node version). This
makes the tests future-proof.
@TheDahv TheDahv mentioned this pull request Apr 11, 2019
@floflock
Copy link

floflock commented May 9, 2019

@TheDahv who can merge this branch?

@TheDahv
Copy link
Author

TheDahv commented May 9, 2019

@floflock I don't know. I haven't been in contact with anyone from Indix.

@TheDahv
Copy link
Author

TheDahv commented May 9, 2019

@floflock oh I just read the other thread. Sounds like he wants us to work from a fork.

In that case, we have 2 options:

  • you can clone this branch into a repo you manage and work from that
  • I can merge into my fork and remember to keep it up to speed occasionally

I suppose it comes down to which of the two of us wants to become a maintainer :/

@floflock
Copy link

floflock commented May 9, 2019

@TheDahv it is up to you. :)
Currently, I am ignoring those type of log spam. 😆

In my opinion, there is more to do: new esm syntax or typescript, more test cases, ...

@raine
Copy link

raine commented Jul 8, 2020

Is there a more actively maintained similar library?

@raine
Copy link

raine commented Jul 8, 2020

I've made a fork and merged the changes from some of the other forks: https://github.com/raine/web-auto-extractor

Published as @rane/web-auto-extractor to npm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in jsonld parse
3 participants