You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Robert Munteanu <ro...@apache.org> on 2021/01/07 16:22:50 UTC

Re: SlingPostServlet should not throw 500 on any exception

Hi Joerg,

On Wed, 2020-12-23 at 11:25 +0100, Jörg Hoh wrote:
> I would love to get some feedback on
> https://github.com/apache/sling-org-apache-sling-servlets-post/pull/7
> , so
> we can get a fix for my problem committed soon.

I had a single comment on the conceptual level, it would be good to
discuss it, and hopefully the others involved will find the time to
review as well.

Thanks,
Robert

> 
> Thanks,
> Jörg
> 
> Am Do., 17. Dez. 2020 um 12:54 Uhr schrieb Jörg Hoh <
> jhoh228@googlemail.com
> > :
> 
> > Thanks Bertrand, I implemented your feedback.
> > 
> > As my initial implementation was deemed to simple and not covering
> > relevant cases, I created a new PR at
> > https://github.com/apache/sling-org-apache-sling-servlets-post/pull/7
> > which tries to return a different status code based on various JCR
> > exceptions.
> > 
> > Thanks
> > 
> > Am Mo., 14. Dez. 2020 um 16:29 Uhr schrieb Bertrand Delacretaz <
> > bdelacretaz@apache.org>:
> > 
> > > Hi,
> > > 
> > > I'm coming late to this discussion, with a smallish thing..
> > > 
> > > On Wed, Nov 11, 2020 at 1:10 PM Jörg Hoh <
> > > jhoh228@googlemail.com.invalid>
> > > wrote:
> > > > ...Having that in mind, I would nevertheless argue to switch
> > > > the
> > > behavior of
> > > > the SlingPostServlet to return a 405 "Method not allowed" in
> > > > the case
> > > of a
> > > > PersistenceError [2]...
> > > 
> > > I would much prefer a 409 "Conflict" status which as per rfc7231
> > > indicates "a conflict with the current state of the target
> > > resource"
> > > where "the user might be able to resolve the conflict and
> > > resubmit the
> > > request."
> > > 
> > > I think it's suitably vague, whereas 405 is meant for when the
> > > HTTP
> > > method used is not appropriate, which is not the problem here
> > > IMO.
> > > 
> > > Also, 405 is indicated to be cacheable by default which is
> > > probably
> > > not what we want here.
> > > 
> > > -Bertrand
> > > 
> > > > [2]
> > > > 
> > > https://github.com/apache/sling-org-apache-sling-servlets-post/blob/master/src/main/java/org/apache/sling/servlets/post/impl/SlingPostServlet.java#L237
> > > 
> > 
> > 
> > --
> > Cheers,
> > Jörg Hoh,
> > 
> > http://cqdump.wordpress.com
> > Twitter: @joerghoh
> > 
> 
>