ArcanePlugins/Treasury

`Response`: add method to fail with a given exception

lokka30 opened this issue ยท 6 comments

Often times, I am writing code to create a failed response caused by a general exception being caught when attempting to do something like retrieving a balance. It would be nice if Response had a static method which creates a failed response with a given exception object.

This could be better implemented as a static method in FailureReason: FailureReason#of(Exception)

I encourage you to read this comment for a better explanation of why this may be useful.

I have labelled this issue as a good first issue, since it should be quite an easy job for a keen first-time contributor.

Please leave a comment describing your opinion on this matter below.

Approvals:

  • @lokka30
  • @MrIvanPlays

    Comment pending; Ivan is busy at the moment.

  • @Jikoo

    Comment pending; Jikoo is busy at the moment.

  • Anyone else who wishes make a justified vote will be added here.

Hi, I found this issue though the 'https://goodfirstissues.com/'. just some experience, we rarely use the entity with exception object in our daily coding work, since it's not necessary. We usually use the following two methods:

  1. status code or error enum, in this case you have to manage it manually, after all, you need to consider all the situations.
  2. custom exception and exception handler, use 'event bus' or interceptor to catch the fatal error and handler it. As a backstop, maybe a global handler is needed.

Perhaps the only sure thing is to consider all error cases , of course a static method is quite helpful
Any other methods please let me know :)

Hi, welcome! Glad to hear that the web embraces the good first issue label. ๐Ÿ˜„

Treasury does not deal with any exceptions itself - it uses a response object which can either 'succeed' or 'fail', with a result or failure reason being provided for those respective outcomes. These Response objects are (at the moment, always) wrapped within a CompletableFuture, which in combination, provides a way for a plugin to make a query to a service provider, which 'will be responded to at some point in the future, and the response may or may not be successful'.

This issue proposes a static method which instantiates a failed Response through supplying a given Exception object.

  • The proposed method's use case is for service providers implementing Treasury who would like to ensure no exceptions leak their way into the CompletableFuture.
  • This may make it easier for service providers to implement the Treasury API.
  • Note that this method is not meant to be used as a lazy way to fail any Response upon an exception, it's meant to be a fallback for unexpected runtime exceptions. Compile-time exceptions should be handled as they currently are, such as if the database is unable to complete a query.

Thanks

exceptions should be handled via futures as there proper code and proper methods exist. if someone makes it so that his code swallows exceptions, not our problem.

exceptions should be handled via futures as there proper code and proper methods exist. if someone makes it so that his code swallows exceptions, not our problem.

So the design will be that unexpected exceptions -> handled by CFs, forced failures by the economy provider -> handled by responses? I'm all good with this, in fact, it may make it easier to implement. Though, I am concerned about having to handle errors in two separate places, guess it's not really a biggie.

exceptions such as sqlexception (when handling with a sql type database) shall be handled by the economy implementation and sent through as failure response, whilst unexpected exceptions (such as NPEs) shall be handled via futures.

Thanks, reshaped this issue into #256 so that the failure behaviour is clarified. ๐Ÿ™‚