uPlot icon indicating copy to clipboard operation
uPlot copied to clipboard

Chrome crash on scale range [1, 1]

Open mario-jerkovic opened this issue 3 years ago • 8 comments

uPlot version: 1.6.17 Browser: Chrome 95.0.4638.69 OS: macOS

Bug description: If you set scale range to equal numbers like [1,1], [2,2] and etc.. except [0,0] memory usage skyrockets and tab crashes. Screenshot 2021-11-16 at 14 08 49

Expected behaviour: In previous versions of uPlot if scale has equal range values it wouldn't crash the tab and scale label would be hidden

To reproduce: In any of the project examples set scale range to [1,1]

mario-jerkovic avatar Nov 16 '21 13:11 mario-jerkovic

I have the same issue, please fix this 🙏

Andro172 avatar Nov 16 '21 13:11 Andro172

will take a look today.

  1. out of curiosity, can this be triggered outside of manually returning a min == max range?
  2. is there a logical expectation that this is a valid range to return, besides that it used to work? like, in what scenario would you need to do this?

leeoniya avatar Nov 16 '21 13:11 leeoniya

@leeoniya

  1. Yes
  2. This will happen when Chart receives one X value and one Y value and scale range function by default will return min: 1 and max: 1, example below:
data: [
    [
        1_636_668_028_800,
    ],
    [
        0, // uPlot range function returns min: 0, max: 0 which is fine
    ],
    [
        1, // uPlot range function returns min: 1, max: 1 which crashes 
    ],
]

mario-jerkovic avatar Nov 16 '21 13:11 mario-jerkovic

Yes

i have a bunch of single-point scenarios here, and none trigger a crash:

https://leeoniya.github.io/uPlot/demos/no-data.html

here is your data without a crash:

https://jsfiddle.net/fy3kapz5/

leeoniya avatar Nov 16 '21 14:11 leeoniya

We have managed to use most of the uPlot option that we have in our application.

  1. Example with our data (timestamps): https://jsfiddle.net/mariojerkovic/8qw64hmz/4/
  2. Example with your data (timestamps): https://jsfiddle.net/mariojerkovic/8qw64hmz/6/

In both examples just change range to [min, max]

mario-jerkovic avatar Nov 16 '21 14:11 mario-jerkovic

right, which is why my first question was whether this happened without you manually returning a nonsense scale. you said "Yes", but your demo is "No". in this case, you are manually returning a nonsense scale range; you can also probably cause a crash by returning ["foo", -5].

uPlot.rangeNum() has these guards built in, and internally that is what scales use unless you supply your own scale.range. in this case it is up to you to return a range that is non-0, numeric, and has max > min.

for now, you should guard against this either manually, or by using uPlot.rangeNum(). e.g. https://github.com/leeoniya/uPlot/blob/1692dc3556a9b8c883abf8488859f4272ceb6ee4/demos/no-data.html#L32-L37

i will consider adding a guard for this specific case to avoid an infinite loop (cause these are hard to debug), but there are a lot of other places in the API when you can do the wrong thing and cause all sorts of unexpected behavior.

leeoniya avatar Nov 16 '21 15:11 leeoniya

I have misunderstood 1st question, sry for that.

Fortunately I cant use ["foo", -5] because of typescript types as they are [min: number | null, max: number | null] but I see your point.

Thanks for the quick response 👍

mario-jerkovic avatar Nov 16 '21 15:11 mario-jerkovic

soon there's going to be some significant internal rework to how scale auto-ranging works to take into account axis tick density requirements and auto-range in a way that ensures there is a tick above the max and below the min (right now this is not guaranteed and can lead to less than ideal situations [1]). some initial work is in [2]

[1] https://github.com/grafana/grafana/discussions/39307 [2] https://github.com/leeoniya/uPlot/tree/ticks-axis-incrs

leeoniya avatar Nov 16 '21 17:11 leeoniya