Geocoder
Geocoder copied to clipboard
feat: save chain exceptions
Relates to #881
Would be interested in your thoughts @geodeveloper @Nyholm @jbelien This allows the chain to execute as it does currently, but adds the ability to handle the exceptions if they occured.
For example, you could log QuotaExceeded
exceptions and ignore UnsupportedOperation
ones.
Example:
$chain->geocodeQuery($query);
if ($chain->getPreviousQueryExceptions()) {
$exceptionsToLog = array_filter($chain->getPreviousQueryExceptions(), function (\Throwable $e) {
return $e instanceof QuotaExceeded;
})
}
Hi @atymic,
Thanks for bringing up this solution. I guess that will work as a workaround. I think the class Chain needs a better approach to handle different scenarios and the final
class should be reconsidered if it's really necessary.
What if we collect exceptions and then if no result we throw either:
- one exception if
count($exceptions) === 1
- A
new ChainExceptionCollection($exceptions)
We do someting similar in Symfony/messanger.
@Nyholm
I'm not sure that would work in this context. The issue is the chain provider only runs until we get a valid result (which it returns), so we'd never have a chance to throw the exceptions.
For example:
Provider 1 -> Exception
Provider 2 -> Exception
Provider 3 -> Success
Results in return of $result from the last provider
Unless we changed the behaviour to throw an exception even if we find a valid result, the only case where we would throw a ChainExceptionCollection
would be when they all fail.
We should not throw exception if we get a result. They should only be thrown if no result was found.
I like the idea of getting exceptions throw a getter even if we do find a result.
We should not throw exception if we get a result. They should only be thrown if no result was found.
I agree. I can add a ChainExceptionCollection
in the case that no results are found, but that would be a BC break hence why I went with this solution (which doesn't break BC)
No it wouldn’t. The contract is from the interface. The interface states that an exception might be thrown, right?
I can add a ChainExceptionCollection in the case that no results are found
I want to clarify. A provider interface says that a provider returns an empty collection when there is no result for the query and no error has happened (e.g. when the given address doesn't exist). Therefore empty result doesn't mean that an exception must be thrown. I think that the chain provider must throw an exception only when no results are retrieved and an inner provider has thrown an exception.
I think that the chain provider must throw an exception only when no results are retrieved and an inner provider has thrown an exception.
I agree with @Finesse.
Well, actually, it depends on what a "chain" provider is intended to be used for. It is here to have a "winner takes all" behavior, or is it here to have a "fault tolerant" behavior?