0xProject / 0x-launch-kit-frontend

Apache License 2.0
112 stars 207 forks source link

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

Closed mariano-aguero closed 5 years ago

mariano-aguero commented 5 years ago

Closes #515

Includes

Console log error image: Screenshot_20190614_182311

How to reproduce the bug

Based on this comment

  1. Set the following values in the .env file:
    REACT_APP_RELAYER_URL='https://api.kovan.radarrelay.com/0x/v2'
    REACT_APP_NETWORK_ID=42
  2. Open the file src/common/tokens_meta_data.ts and modifle line 44 so that the address is wrong (for example, copy the value from the Network.Ganache).
unjapones commented 5 years ago

@mariano-aguero I was not able to reproduce this bug, even by doing the following (using this PR's branch or development):

  1. getting 0x-launch-kit-backend (latest from development).
  2. setting WHITELISTED_TOKENS to ZRX and MKR addresses and explicitly leaving out WETH.

The backend always answered with a 200 response with total: 0. I think the real reason of the problem was mistaken for for one of the problems that the lack o a default networkId generated.

On the other hand, this PR introduces some improvements:

  1. the PR kind of makes the thunk in src/store/market/actions.ts more robust.
  2. changes added by this PR to src/util/known_tokens.ts do not fix one of the things commented in the original issue(https://github.com/0xProject/0x-launch-kit-frontend/issues/515#issuecomment-497997849), but adds more details to the exception being thrown.

I think the best things to do here are:

P/s: if someone was able to reproduce the bug #515 and confirms that the code in this PR fixes it, just forget everything that I wrote above :grimacing:

mariano-aguero commented 5 years ago

@mariano-aguero I was not able to reproduce this bug, even by doing the following (using this PR's branch or development):

  1. getting 0x-launch-kit-backend (latest from development).
  2. setting WHITELISTED_TOKENS to ZRX and MKR addresses and explicitly leaving out WETH.

The backend always answered with a 200 response with total: 0. I think the real reason of the problem was mistaken for for one of the problems that the lack o a default networkId generated.

On the other hand, this PR introduces some improvements:

  1. the PR kind of makes the thunk in src/store/market/actions.ts more robust.
  2. changes added by this PR to src/util/known_tokens.ts do not fix one of the things commented in the original issue(#515 (comment)), but adds more details to the exception being thrown.

I think the best things to do here are:

  • edit this PR's description (remove the `Closes #515) part and describe the improvements.
  • leave a comment in the original issue ("Broken market drodpdown ...") to check if this still happens in development. If the erronueous behavior is not observed anymore, just close the bug-issue.
  • open a different issue for the problem observed in the comment: yes, the application breaks very badly but that would happen if someone copy and pasted the URL in a new browser tab and introduced a typo.

P/s: if someone was able to reproduce the bug #515 and confirms that the code in this PR fixes it, just forget everything that I wrote above

I was able to reproduce the bug with the following steps: 1 - Add in src/util/types.ts a new token symbol like this:


export enum TokenSymbol {
    Weth = 'weth',
    Zrx = 'zrx',
    Dai = 'dai',
    Mkr = 'mkr',
    Rep = 'rep',
    Dgd = 'dgd',
    Mln = 'mln',
    Xyz = 'xyz',
}

2 - Use this configuration for the file src/common/markets (Add Xyz token, doesn't exist):

import { CurrencyPair, TokenSymbol } from '../util/types';

export const availableMarkets: CurrencyPair[] = [
    {
        base: TokenSymbol.Zrx,
        quote: TokenSymbol.Weth,
    },
    {
        base: TokenSymbol.Mkr,
        quote: TokenSymbol.Weth,
    },
    {
        base: TokenSymbol.Xyz,
        quote: TokenSymbol.Weth,
    },
];

With this configuration you will be able to reproduce the problem.

Regards