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

metaplex.nft.findAllByOwner blows up with async exception #79

Open
clarkezone opened this issue Nov 6, 2022 · 11 comments
Open

metaplex.nft.findAllByOwner blows up with async exception #79

clarkezone opened this issue Nov 6, 2022 · 11 comments
Assignees

Comments

@clarkezone
Copy link

clarkezone commented Nov 6, 2022

Describe the bug

I'm attempting to convert https://github.com/clarkezone/NATIV to use the metaplex android APIs from hand coded json parsing as suggested by @creativedrewy. I've copied the basic code samples from this repo to attempt to enumerate NFTs for a public address but the findAllByOwner method is currently blowing up with an async exception.

To Reproduce

  1. git clone https://github.com/clarkezone/NATIV.git
  2. open solution in Android Studio
  3. open up SolanaNFT/src/main/java/com/creativedrewy/solananft/metaplex/MetaplexNftUseCase.kt
  4. Put a breakpoint on line 40: metaplex.nft.findAllByOwner(ownerPublicKey).apply {
  5. Launch android studio debugger and confirm breakpoint is hit
  6. Step over breakpoint

Expected behavior

Method call succeeds or else the onfailure block is hit

Actual behavior

Unhandled async exception. When launching without debug, app exits

Screenshots

If applicable, add screenshots to help explain your problem.

SDK Version & Context

Platform: Android
Version 1.2.0
Additional context

@Funkatronics
Copy link
Contributor

Hi @clarkezone I am not able to reproduce this issue on my end. Can you please add a minimal reproducible example?

@Funkatronics
Copy link
Contributor

I've added a bug report template, would you mind reformatting/reopening the issue with this template? this is a great help to us and will expedite the fix for you

@Funkatronics Funkatronics self-assigned this Nov 6, 2022
@clarkezone
Copy link
Author

I will see what i can do today, and will also look at bug template. The repo in the initial bug report has the code / state that reproduces the problem BTW

@clarkezone
Copy link
Author

OK I updated the bug above with a set of steps. LMK if you can repro based on those.

@Funkatronics
Copy link
Contributor

Nice, really appreciate the detailed report. I am flying back from Lisbon to the states tonight but I will check this out in the next day or two and try to get a fix out!

@clarkezone
Copy link
Author

Awesome thx, me too! Once you track down issue any debugging techniques that helped would be appreciated for tracking this kind of Async exception on android down in future to provide more targeted report and or PR with fix ;-)

@Funkatronics
Copy link
Contributor

I'd have to look deeper into your application code but async exceptions like this are typically do to mismanagement of the error within an unconfined coroutine context. I see that your app is using the default dispatcher and RPC driver. Is there a reason why you are not using the RpcRequestClient as your RPC driver for the NftClient? It looks to me like it oculd easily be extended to implement the JsonRpcDriver interface. This is is probably not necessary but just trying to understand more about the app architecture here.

I am trying to create a minimal reproduceable example so I can determine if this is an error in the library error handling, or an error in your app code, but I am not finding anything. We do have some gross error wrapping in the Nft Client that I need to clean up, but so far in my testing the findAllByOwner method is handling errors properly and always returns a Result.Failure. I am also pretty sick right now with a head cold so not in my best mental state tbh 😅

In order to nail down exactly where the error is occurring, I would recommend unit testing the MetaplexNftUseCase class within the SolanaNFT module. Unit tests should always be the first line of defense, as trying to debug this kind of error in the app itself is really difficult. Some unit tests would help you find out where in the stack the error is occurring, which would then help me assist you further.

Also, have you tried using the latest beta release instead of 1.2.0? Not much has changed within the findAllByOwner method so I doubt this will fix it for you, but still best to test against the latest code IMO.

@Funkatronics
Copy link
Contributor

Really do appreciate your report here tho, and I want to help you track down the error so we can improve the library and make it easier to use. I think adding some testing with flows on our end would be good so we can ensure everything works (like error handling) when using the client in a flow context.

@clarkezone
Copy link
Author

I forked this project at Breakpoint based on a conversation with @creativedrewy who's one of the devs on Solana Mobile with the intent of replacing his hand rolled json parsing with the Metaplex SDK (again, his suggestion). I don't have a lot of experience with Android dev so can't speak to your questions easily about dispatcher choices, was intending to make the most minimally invasive change possible to his app, but if there is a need to wholesale change the dispatcher model I could look at that after some heavy consultation with docs :-)

@clarkezone
Copy link
Author

BTW thanks for looking and trying to help out here, really appreciate it!

@clarkezone
Copy link
Author

I found some quite good docs on coroutines and dispatchers, I'll see if i can make a unit test for that class as you suggest. Also, hope you feel better sorry about the head cold!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants