On 15 Mar 2014, at 7:22 am, Marcin Erdmann <marcin.erdm...@proxerd.pl> wrote:
> > > > On Fri, Mar 14, 2014 at 7:56 PM, Adam Murdoch <adam.murd...@gradleware.com> > wrote: >> >> Yet, there is plenty to be done on error handling and making sure all >> resources are being released even in error scenarios - so the boring but >> important parts. >> >> I have one question about error handling. There are three types of failures >> listed in the spec: invalid credentials, cannot connect to the server and >> everything else (called server exception). I think that we probably want to >> extract all permission related errors (when both resolving and uploading) as >> a separate error type so that the error message provides user an idea that >> the error is permission related as it feels that it might be a common error. >> What do you think? > > The key is to make the error message informative at this stage, which we > don’t necessarily need a new type for. If we formalise ‘not authorised’ > problems in the resource APIs then we would also need to update the file and > http backed resource implementations to. So, I’d leave this as a later > refactoring. > > What I meant here by a new type is not a new exception class but to > differentiate permission errors from generic errors when it comes to what > message is being shown when such an error occurs. At the moment I'm throwing > a new runtime exception of type SftpException with a message saying what has > happened (invalid credentials, connection error) and where (host and port > part of the url). I was asking if we should check if we have permissions to > perform a given operation (file download or upload, directory creation, etc) > and if not throw a SftpException with a message saying that permission was > denied to do it. It depends how you’re proposing to implement it. If it’s something that we can figure out from the response to the operation, then that makes sense. If it's something we figure out by doing another request prior to the operation, then that’s less interesting, as it’s a potential performance problem, plus the answer can change in the window between the 2 requests. One alternative to this would be to do the permission check only if the original operation fails for some opaque reason, to provide better diagnostics. -- Adam Murdoch Gradle Co-founder http://www.gradle.org VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting http://www.gradleware.com
