OOM error caused by JSON-LD errors

[antoineeripret]

Hi @eliasdabbas,

If you crawl a website that happens to have a lot of JSON-LD parsing error, the following is triggered (source)

except Exception as e:
            jsonld = {"jsonld_errors": str(e)}
            self.logger.exception(
                " ".join([str(e), str(response.status), response.url])
            )

Which can lead to an OOM because the jsonld includes the full error string. If you have a dozen of pages, it doesn't really matter, but if you have more and a lot of errors, it does.

Do we really need to store the full error? Can't we use something simpler?

Happy to provide a PR, but I wanted to discuss the approach with your first :)

[antoineeripret]

FWIW, the error in my case comes from empty JSON ...

<script type="application/ld+json">
 
</script>

... so we could easily add a fix for this case with a built-in check in your function, but I wanted to share the bigger picture with you :)

[eliasdabbas]

@antoineeripret always great to get issues coming from heavy usage.

This is a simple business decision actually: When implementing crawl(keep_columns, discard_columns), I set:

always_include = {"url", "errors", "jsonld_errors"}

advertools/advertools/spider.py

Line 525 in 035e7cb

always_include = {"url", "errors", "jsonld_errors"}

Even if you add any of them to the discard_columns parameter they will still be included in the output_file. This is just to make sure you get the full view (a long lis of URLs with empty pages, is not informative without error messages).

JSON-LD errors are slightly different, because you can actually crawl a full page, even if you have JSON-LD errors. A very easy fix would be to remove "jsonld_errors" from always_include, and then you, as a users would discard_ that column from the output_file.

This is my initial thinking, I'm not sure this is the best approach, but curious to now what you think.

[antoineeripret]

Hi @eliasdabbas,

That would be an amazing solution and easier than what I initially thought !

Do you want me to prepare the PR? Is there any specific piece of documentation that needs to be updated after this change, or is this an "hidden" behavior?

Thank you !

[eliasdabbas]

@antoineeripret Cool. That would be great.

In terms of documentation, I just realized that the keep_columns and discard_columns parameters are not written about in the main documentation page. If you're interested, a paragraph can be added to showcase how this functionality can be used, also inside the function itself, we can simply add a very short line about the fact that only url and errors columns will be kept even if you discard_columns them.

What do you think?