Skip to content

feat(exception): add InvalidDataThrowable marker interface - #6

Merged
ralflang merged 1 commit into
FRAMEWORK_6_0from
feat/invalid-data-throwable
Nov 2, 2026
Merged

ralflang merged 1 commit into
FRAMEWORK_6_0from
feat/invalid-data-throwable

Conversation

@TDannhauer

@TDannhauer TDannhauer commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds the marker interface Horde\Exception\InvalidDataThrowable (extends HordeThrowable), following the PermissionDeniedThrowable pattern.

Motivation

Applications throw their own exception classes when a caller supplies data they cannot store (a Turba attribute over its configured maxlength, for example). Protocol handlers in other packages — Horde_Core_ActiveSync_Driver in this case — need to distinguish "the client's data is invalid, do not retry" from "the server failed, try again" without depending on Turba_* classes. A framework-level marker interface keeps that check implementation-agnostic.

Context: horde/turba#73 (attribute constraints only enforced in the web form; ActiveSync clients resend a rejected contact forever because it is reported as a server error).

Changes

  • src/InvalidDataThrowable.php — empty marker interface with docblock describing the contract (the exception message describes the invalid input; retrying with the same input will fail again).

Related PRs

Note for release: adding the interface is a minor-version feature; the dependent packages currently require horde/exception: ^3 || dev-FRAMEWORK_6_0 and will need ^3.1 (or whichever version ships this) once released.

Test plan

  • php -l src/InvalidDataThrowable.php
  • Consumers tested in the Turba and Core PRs (Turba_Exception_InvalidAttribute instanceof InvalidDataThrowable)

Marks exceptions caused by invalid data supplied by the caller (as
opposed to a failure of the server), so consumers such as protocol
handlers can tell "do not retry this item" from "try again later"
without depending on the concrete exception classes of an application.

Same pattern as PermissionDeniedThrowable. First implementer is
Turba_Exception_InvalidAttribute (horde/turba#73).
@ralflang

ralflang commented Nov 2, 2026

Copy link
Copy Markdown
Member

I am not against it but I wonder if we are not better off basing this on either InvalidArgumentException or UnexpectedValueException (a runtime exception which says "this value doesn't fit the constraints"

@ralflang
ralflang merged commit 0d76f8f into FRAMEWORK_6_0 Nov 2, 2026
0 of 2 checks passed
@TDannhauer

Copy link
Copy Markdown
Contributor Author

Agreed that UnexpectedValueException is the natural SPL fit — your docblock on HordeUnexpectedValueException describes exactly this case ("typically from external sources like … user input … a data validation failure").

The reason for a marker interface instead of a base class is Turba_Exception: it extends Horde_Exception_Wrapped, and 42 catch (Turba_Exception $e) sites in turba/core (API, CardDAV, import, ActiveSync driver) must keep catching the new Turba_Exception_InvalidAttribute. With single inheritance it cannot also extend UnexpectedValueException. The same holds for every other legacy <App>_Exception. The interface lets an app add the "client data, do not retry" classification on top of its existing hierarchy — same pattern as PermissionDeniedThrowable.

The two are not exclusive: once the Turba exceptions move to src/, extends HordeUnexpectedValueException implements InvalidDataThrowable would give us both the SPL semantics and the retry-policy marker.

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.

2 participants