search

contentful / rich-text.php

Utilities for the Contentful Rich Text
https://www.contentful.com
MIT License
12 stars 9 forks source link

Fix/44 entry references is hiding bugs #46

Closed pgrigoruta closed 4 years ago

e1himself commented 4 years ago

https://github.com/contentful/rich-text.php/issues/44#issuecomment-582965457

The MapperException repackaging can indeed catch unintended behaviour. In PR #46 (which should be merged soon), I am adding the original exception to the Mapper Exception. This should be semantically correct, i.e. everything generated by that part of code is a Mapper Exception, but now you have more information as into what it was, so you can write conditional code on your client.

@pgrigoruta Let me disagree with this solution. It does not fix #44 at all.

Catching these exceptions on client is not possible, as any MapperException is caught and resolved to a Nothing by Parser.

https://github.com/contentful/rich-text.php/blob/82a269192d5815d65677e7e01d91577d8e06006f/src/Parser.php#L64-L70

These exceptions never hit client code. That's the problem. We cannot catch them, we cannot even know if an exception happened. Any error will silently be converted into a Nothing node.

We have hard time debugging problems with random elements disappearing on our website pages without any error logged. Probably it's caused by temporary network issues happening during resolving links. But the problem -- we don't know and we have no means to find it out with the current library implementation.

e1himself commented 4 years ago

For future reference: this is fixed with #47