Add handling custom error class #5
Conversation
| if (preg_match('/.*\[\]$/', $className)) { | ||
| $className = substr($className, 0, -2); | ||
| } |
There was a problem hiding this comment.
Is it needed? Seems to be checking if the $className ends with a [], which won't be a case here - yet it was supported for response classes.
There was a problem hiding this comment.
I think it was introduced to avoid a situation when the configuration is put as an array in YML. I would leave it as it is to keep the things the same
| if (null === $errorClass) { | ||
| return $endpoint; |
There was a problem hiding this comment.
Wouldn't it be easier to always have an $errorClass ? So that a default would be defined as well for generic purposes/existing implementations?
There was a problem hiding this comment.
Error class like other classes are in the ServiceAPIClientBundle repository. I'm worried about circular dependency.
There was a problem hiding this comment.
Error class like other classes are in the ServiceAPIClientBundle repository. I'm worried about circular dependency. Let's keep it that way
| @@ -58,6 +63,7 @@ class EndpointImmutable implements EndpointInterface | |||
| * @param string $responseFormat | |||
| * @param null|string $responseClass | |||
| * @param null|string $dateTimeFormat | |||
| * @param null|string $errorClass | |||
There was a problem hiding this comment.
general things about implementation:
-
easy part: as mentioned, please introduce errorFormat in alignment with responseFormat (json by default but overridable)
-
less easy part: response returns either particular entity or collection of them
errors aren't so straightforward
would love to see the configurable collection of objects here with error codes as key
because in most cases responses would be smth like:- 400\409: validation error collection
- 401\403\404 - dummy body
- 500: some less dummy body, maybe
please think it through
No description provided.