knowm / XChange

XChange is a Java library providing a streamlined API for interacting with 60+ Bitcoin and Altcoin exchanges providing a consistent interface for trading and accessing market data.
http://knowm.org/open-source/xchange/
MIT License
3.85k stars 1.94k forks source link

Kucoin unable to load ExchangeMetaData, prevents start up #4674

Open nibarulabs opened 1 year ago

nibarulabs commented 1 year ago

I'm still digging through the code, but I think something with the new Instrument code broke Kucoin.

For Binance.us, I was seeing a 'warning' message on startup:

An exception occured while loading the metadata file from the file system. 
This is just a warning and can be ignored, but it may lead to unexpected results, so it's better to address it

That error is from BaseExchange.java:148

When I saw this on startup with Binance, I ignored it was marked as a warning. But, it appears it's fatal - for at least Kucoin. What I think is happening is KucoinExchange.java:94 throws an exception because in the KucoinAdapters.adaptMetadata method, the debugger gets an npe on line 133. That line basically tries to call getInstruments() from a null ExchangeMetaData object.

  public static ExchangeMetaData adaptMetadata(
      ExchangeMetaData exchangeMetaData,
      List<CurrenciesResponse> currenciesResponse,
      List<SymbolResponse> symbolsResponse,
      TradeFeeResponse tradeFee)
      throws IOException {

>>  Map<Instrument, InstrumentMetaData> currencyPairs = exchangeMetaData.getInstruments();
    Map<Currency, CurrencyMetaData> currencies = exchangeMetaData.getCurrencies();
    Map<String, CurrencyMetaData> stringCurrencyMetaDataMap =
        adaptCurrencyMetaData(currenciesResponse);

I haven't had a chance to look through the rest of the exchanges, but any others that follow this pattern are probably broken as well.

The culprit for all this appears to be the InstrumentDeserializer. When it tries to deserialize the exchange json file in the jar, it throws this error:

Cannot find a (Map) Key deserializer for type [simple type, class org.knowm.xchange.instrument.Instrument]

When I switched over the deserialize annotation to

@JsonDeserialize(keyUsing = InstrumentMapDeserializer.class)

I now get the error

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of 
`org.knowm.xchange.instrument.Instrument` (no Creators, like default constructor, exist): 
abstract types either need to be mapped to concrete types, have custom deserializer, or 
contain additional type information

Looking back through the history, CurrencyPair was a concrete class where Instrument is an abstract class and Jackson fails on that, even with the deserializer class. I would note too that all the xchange-core tests pass with just the existing @JsonDeserialize(using = InstrumentDeserializer.class) annotation, so I could see how this was seen as ok. The Kucoin tests don't specifically test loading in the json exchange metadata, so that might be something to add in the future.

I don't know enough of the ins and outs of Jackson to know what to do at this point but I'll keep digging and seeing if I can find a solution. Maybe @timmolter and @makarid might want to take a look?

nibarulabs commented 1 year ago

I have develop checked out and just added a quick test to the xchange-kucoin module. I basically just grabbed the code out of the BaseExchange that loads the metadata file to see if I could duplicate the parse error.

Of course, it worked no problem. My project is using the latest 5.1.0 release for everything. I'm using 11.0.17 GraalVM.

Definitely scratching my head on this one now.

makarid commented 1 year ago

Hello, probably Kucoin module didn't had some tests in order to check that the parsing of instruments is being done correctly, that is why the new InstrumentsMetadata update didn't find the problem. I don't work with Kucoin exchange, so I cannot know what has been broken. But what I have found in other exchanges is that, it's possible some pairs do not have a specific metadata info, let's say volumeScale for example and if some calculation is being done on the adaptMetadata function on volumeScale, it throws NullPoint exception. The other that comes to mind is that the kucoin.json file need to updated to support the new InstrumentsMetadata. I hope it helps.

On Sat, Feb 4, 2023, 1:38 AM Nibaru @.***> wrote:

I have develop checked out and just added a quick test to the xchange-kucoin module. I basically just grabbed the code out of the BaseExchange that loads the metadata file to see if I could duplicate the parse error.

Of course, it worked no problem. My project is using the latest 5.1.0 release for everything. I'm using 11.0.17 GraalVM.

Definitely scratching my head on this one now.

— Reply to this email directly, view it on GitHub https://github.com/knowm/XChange/issues/4674#issuecomment-1416531933, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHIWQ74PI7LNSWWYH7PAROTWVWJHPANCNFSM6AAAAAAUQW2DWA . You are receiving this because you were mentioned.Message ID: @.***>

nibarulabs commented 1 year ago

@makarid The Kucoin module definitely didn't have any tests covering this, but I think the issue is changing from a concrete CurrencyPair class to an abstract Instrument class.

We can see here when I start up our product Jackson is trying to deserialize for Instrument and it cannot.

2023-02-04 08:47:48 WARN KucoinExchange - An exception occured while loading the metadata file from the
file system. This is just a warning and can be ignored, but it may lead to unexpected results, so it's better
to address it. com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a (Map) Key 
deserializer for type [simple type, class org.knowm.xchange.instrument.Instrument]

If you look at the error, it says it cannot find the Key deserializer for Instrument - I don't read that as anything specific in the content (json) trying to be read. When I went into the xchange-core code and changed the annotation to keyUsing instead of using, a bunch of the tests would not pass. The tests started complaining about more deserialization problems, like Instrument lacking a @JsonCreator annotation, etc.

At this point, I don't know what ramifications this has for everything now. I've committed our code base to 5.1.0 and while I don't have this issue with a patched Binance.us module, Kucoin is breaking and I will just have to handle the exception in our code and figure out how to manually initialize the exchange meta data and move on.

If I have time, I can come back to this and dig more.

makarid commented 1 year ago

@makarid The Kucoin module definitely didn't have any tests covering this, but I think the issue is changing from a concrete CurrencyPair class to an abstract Instrument class.

We can see here when I start up our product Jackson is trying to deserialize for Instrument and it cannot.

2023-02-04 08:47:48 WARN KucoinExchange - An exception occured while loading the metadata file from the
file system. This is just a warning and can be ignored, but it may lead to unexpected results, so it's better
to address it. com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a (Map) Key 
deserializer for type [simple type, class org.knowm.xchange.instrument.Instrument]

If you look at the error, it says it cannot find the Key deserializer for Instrument - I don't read that as anything specific in the content (json) trying to be read. When I went into the xchange-core code and changed the annotation to keyUsing instead of using, a bunch of the tests would not pass. The tests started complaining about more deserialization problems, like Instrument lacking a @JsonCreator annotation, etc.

At this point, I don't know what ramifications this has for everything now. I've committed our code base to 5.1.0 and while I don't have this issue with a patched Binance.us module, Kucoin is breaking and I will just have to handle the exception in our code and figure out how to manually initialize the exchange meta data and move on.

If I have time, I can come back to this and dig more.

I just run the test with the name of testGetMarketData on the test folder without any issue. Do you use the latest version of develop branch? Thanks

nibarulabs commented 1 year ago

@makarid Yah, I'm using latest develop. I mentioned earlier in the thread that I created a quick test in the kucoin module to read the kucoin.json in - just copy and pasted the code from the BaseExchange class that loads that metadata. I wrapped that in a test and it parsed everything fine.

It has to be something in my environment/project then. At this point, it's an effort to go through and update everything in our pom to match (as close as possible) XChange versions. So I'll just make a workaround for Kucoin and see along the way if I can figure out why this is an issue at all. Thanks.

makarid commented 1 year ago

If you solve the issue please close the thread.Thanks

kunalbarchha commented 1 year ago

I have the same issue with Houbi, Binance, as well as Kraken.... Trying to subscribe to market data. I am using OpenJDK 8 with Spring boot 1.5.9, my Guava version is 18, I am using latest 5.1.0 release of XChange core, and other exchange modules as per my requirement.