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: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to