Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPE in TransactionsApi.getTransactionById() if transaction does not exist #28

Open
marcin-kamionowski opened this issue Sep 4, 2018 · 9 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@marcin-kamionowski
Copy link

marcin-kamionowski commented Sep 4, 2018

Driver version: 1.1 and master(cae61d8)
Response:
{"message":"Not found","status":404}
Stacktrace:

Exception in thread "main" java.lang.NullPointerException
at com.bigchaindb.json.strategy.TransactionDeserializer.deserialize(TransactionDeserializer.java:50)
at com.bigchaindb.json.strategy.TransactionDeserializer.deserialize(TransactionDeserializer.java:23)
at com.google.gson.internal.bind.TreeTypeAdapter.read(TreeTypeAdapter.java:69)
at com.google.gson.Gson.fromJson(Gson.java:888)
at com.google.gson.Gson.fromJson(Gson.java:853)
at com.google.gson.Gson.fromJson(Gson.java:802)
at com.google.gson.Gson.fromJson(Gson.java:774)
at com.bigchaindb.util.JsonUtils.fromJson(JsonUtils.java:133)
at com.bigchaindb.api.TransactionsApi.getTransactionById(TransactionsApi.java:74)

@marcin-kamionowski marcin-kamionowski changed the title NPE in TransactionsApi.getTransactionById() if transaction does not exists NPE in TransactionsApi.getTransactionById() if transaction does not exist Sep 4, 2018
@innopreneur innopreneur added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Sep 5, 2018
@kai18
Copy link
Contributor

kai18 commented Oct 14, 2018

Hi, I would like to take up this issue. Kindly let me know if help is still required.

@innopreneur
Copy link
Contributor

innopreneur commented Oct 14, 2018

@kai18 sure. any help is most welcome. Thanks for your contribution. Once you have fixed this issue. Send a PR to this repo. Cheers!

@kai18
Copy link
Contributor

kai18 commented Oct 14, 2018

@innoprenuer Thanks! Just so we are clear, there needs to be an exception hierarchy that forces the user to handle this particular scenario. Right? That is what we are expecting here?

@innopreneur
Copy link
Contributor

innopreneur commented Oct 14, 2018

@kai18 This exception happens because we are expecting valid transaction object but in case of valid response (404, 500) we don't get transaction object and so deserialization doesn't work as there is no check for error codes. So, solution should go somewhere here - take a look here. We can maybe check for error codes and throw 'TransactionNotFound' exception if error code is 404.

@kai18
Copy link
Contributor

kai18 commented Oct 14, 2018

@innoprenuer Thanks for the confirmation! I have raised the PR. Please review and let me know of any changes.
https://github.com/bigchaindb/java-bigchaindb-driver/pull/38

@kai18
Copy link
Contributor

kai18 commented Oct 15, 2018

@innoprenuer Adding this checked exception breaks backwards compatibility, as, when people upgrade, they'll be forced to handle this exception. I was just wondering, maybe we should do this across APIs in a single go? If that's the case, please let me know of any changes in my current PR, and I can do the same for all APIs

@kai18
Copy link
Contributor

kai18 commented May 17, 2019

@ttmc @innoprenuer Is this project still under active development?

@kai18
Copy link
Contributor

kai18 commented May 23, 2019

thanks for the info @ttmc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants