henrykuijpers commented on code in PR #61:
URL:
https://github.com/apache/sling-org-apache-sling-api/pull/61#discussion_r2160407552
##########
src/main/java/org/apache/sling/api/resource/PersistenceException.java:
##########
@@ -91,4 +95,34 @@ public String getResourcePath() {
public String getPropertyName() {
return this.propertyName;
}
+
+ /**
+ * Returns a message for this exception.
+ * If the cause is not null, it will be appended to the message.
+ * @return The message for this exception
+ */
+ @Override
+ public String getMessage() {
Review Comment:
I think something can be done in SlingPostServlet to provide more
information about the failure.
The other question would be: Do we actually *want* to expose this
information? Isn't it fine to have this detailed information about the system
(that is a blackbox for / transparent to the user) in the logs and just let the
user know that something went wrong internally?
Indeed,
https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/fe35d53a1b0f8ff1a7616909f16539b759bd0bdd/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java#L540
is also wrong to me, it should provide additional information and context as
to why this PersistenceException was thrown, which is coming from an
IllegalArgumentException. It should at least tell that it was trying to provide
properties which were not ignored, specifically what key and value as that
would tell a developer immediately which property was causing the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
