Broken dropdown If one token is not supported by the backend

0xProject / 0x-launch-kit-frontend

Apache License 2.0

114 stars 208 forks source link

Broken dropdown If one token is not supported by the backend #515

Closed dekz closed 5 years ago

dekz commented 5 years ago

If the Backend supports ZRX/WETH, ZRX/DAI, MKR/DAI, but not XYZ/WETH then the entire market dropdown will fail to render correctly.

We should gracefully handle the missing token in a better way, perhaps by showing a Market not ready/configured error.

icinsight commented 5 years ago

If you feed something like http://*.*.*.*:3001/#/erc20/?base=zrx&quote=xyz as an url the whole system crashes. It is not only about the drop-down not rendering. Frame-masking the address helps a bit.

unjapones commented 5 years ago

@dekz can you confirm that the problem in the description of this issue still happens using development?

I wasn't able to reproduce it: I was expecting that some HTTP error response from the backend (originated by a request with a non-whitelisted token) caused an unhandled exception on the frontend.

unjapones commented 5 years ago

@icinsight

If you feed something like http://_._._._:3001/#/erc20/?base=zrx&quote=xyz as an url the whole system crashes. It is not only about the drop-down not rendering. Frame-masking the address helps a bit.

That will happen if the user copies and pastes the URL with a typo. The frontend should gracefully treat that case, but I don't think we ever thought about handling that scenario (mainly because this was a reference implementation, not a full webapp covering all cases).

Do you mind logging an issue to handle this scenario? Will probably require a proper message/404 UI state design.

dekz commented 5 years ago

@unjapones the way this was tested was using Kovan Radar SRA endpoint. Since this endpoint only supports a set of tokens, not all tokens. https://api.kovan.radarrelay.com/0x/v2

unjapones commented 5 years ago

@dekz thanks for the tip to reproduce the bug. Should be kind of worked-around now:

the whole dropdown should not fail to render thanks to the linked PR
the corresponding unsupported markets rows will not be rendered, but the requests will still happen anyway (:-1:)

Regarding the 2nd item, I think it is a configuration thing: if the user configures the DApp the correct way everything should be good. If he/she doesn't do it right... the optimal path should be "query the relayer if the X Asset is not supported... and bla" (but I think this should be considered a feaure and add the corresponding issue).