System.ArgumentOutOfRangeException in ProgressBar.Refresh()

[PromoFaux]

(After writing this up, I actually searched and this may be related to: #20 #33 (though It's not the user resizing the screen, it's RDP)

Edit: Tangentially related to #89, too. I have not seen that one again since reporting, though.

---

Hello there, come up against an issue when calling .Refresh(), that very occasionally throws an out of range exception

Event log shows:

Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.ArgumentOutOfRangeException
   at System.String.CtorCharCount(Char, Int32)
   at Konsole.ProgressBarSlim.Refresh(Int32, System.String)
   at DharaImport.CLI.Program.Main()

Exception thrown in CtorCharCount seems to be when count is negative:

https://github.com/microsoft/referencesource/blob/3b1eaf5203992df69de44c783a3eda37d3d4cd10/mscorlib/system/string.cs#L1661

And I think the part of your code that even hitting that .Net call is:

konsole/src/Konsole/ProgressBarSlim.cs

Line 106 in a070ce7

? new string(_character, (int) ((decimal) (barWidth)*perc)).PadRight(barWidth)

The application I have using this library is a console application that sits in a session on a remote machine that multiple people access. My current hypothesis is that it is something to do with the window being resized when someone connects with a client with a much smaller resolution than normal (i.e RDP on a phone)

Obviously this is an edge case, and I think it's more to do with something that is going on on my end rather than yours, but figured I would report to you in case there were any opportunities to safeguard against the exception from your side (for now I'm just going to wrap all calls to Refresh in a try catch and actually handle the exception 🙃)

[goblinfactory]

Hi Adam
What version of Konsole are you using?
I believe I've added some additional support for similar issues (the one's you've mentioned) in the latest alpha releases.
Alpha is actually quite stable; the only reason they're alpha releases is because there are some API changes that I had not made a final decision to stabilise, but if you start using it and it solves your problem I can easily release the alpha as a new version, and then start a new alpha if there is any further reason to change the API again.

The breaking changes are very minor, and are compile time changes, behaviour stays the same, with new behavior around quite advanced styling added to windows etc.

Since you have a released product I would suggest for that project you simply wrap the call, and for other projects consider using alpha from the start.

Lastly;

instead of wrapping the call inside a try catch, just write an extension method, something like

public static class TempSafeCaller {
 public static SafeRefresh ( this Progressbar ... )
}

or similar. I know it's obvious, but just mentioning it in case for some reason you thought you'd have to make you code all ugly with lots of try { ...Foo.Refresh().. } catch { ...} calls everywhere?

[goblinfactory]

Hi Adam / @PromoFaux

Just checked the code in alpha branch;
And there is a try catch around the progressBarSlim refresh, Unfortunately I think I see the bug,
There's this code in ProgressBarSlim()

        internal void _Refresh(int current, string itemText)
        {
            var item = itemText ?? "";
            var clippedText = item.FixLeft(TextWidth);
            _item = item;
            var state = _console.State;
            _current = current.Max(Max);
            try
            {
                decimal perc = _max == 0 ? 0 : (decimal)_current / (decimal)_max;
                int barWidth = _console.WindowWidth - (TextWidth + 8);
                var bar = _current > 0
                    ? new string(_character, (int)((decimal)(barWidth) * perc)).PadRight(barWidth)
                    : new string(' ', barWidth);
                var text = string.Format("{0} ({1,-3}%) ", clippedText, (int)(perc * 100));
                _console.CursorTop = _y;
                _console.CursorLeft = 0;
                _console.ForegroundColor = _c;
                _console.Write(text);
                _console.ForegroundColor = ConsoleColor.Green;
                _console.Write(bar);
                _line = $"{text} {bar}";
            }
            finally
            {
                _console.State = state;
            }

        }

While this looks safe, there's a custom setter for State that resets the cursor positions back to where they were before refersh was called, if the progress bar is busy refreshing when the window is resized, or opened to be smaller (i.e. no window) then this is probably the culprit. I need another try catch around the state reset.

I'll add that and submit a patch to the latest release, ...
oh (shoot)... I don't have a windows dev machine! lol

my dev machine went in to have it's battery replaced and I formatted it, I've been programming only in Golang and javascript for the last 6 months, lol!

Please use the workaround (add your own extension method) and call "safeRefresh()" for now, and I'll patch the current release and the alpha as soon as I get a working windows machine back up. I've been meaning to do that for a few weeks now taht i got my laptop back.

Last point;

A better fix is to apply the try catch around the State setter, no refresh, that will fix other usages of code that reset's state as well, ...mmm, yes, that's the right fix.

Txs for the detailed bug report! Much appreciated.

p.s. if you're using Konsole for a company for commercial purposes, could you please consider getting your company to sponsor me on Github sponsors, I don't have a single sponsor which very loudly tells me that the project is not important enough to support. I've brought out 22 releases and put in a huge amount of effort thinking it will eventually gain traction or be supported. And well, ... almost nothing. So ... I've stopped working on Konsole.

If I get one sponsor, I'll bring out a new release with various patches, to stabilise the current code base. There are higher sponsorship levels which would I'd offer priority support; that's always available.

If I get 3 sponsors, I'll commit to bringing out major updates to Konsole which will include keyboard input, and the ability to create themable interactive similar to this;

For more background on why I've (temporarily?) stopped working on C# open source, plus links to other similar libraries ... please see the leatest discussion in Gitter here : https://gitter.im/goblinfactory-konsole/community

:D

happy coding! thanks for using Konsole.
regards,

Alan

[PromoFaux]

What version of Konsole are you using?

6.2.2 from nuget, though I see there are now some 7.x alpha packages up there...

Since you have a released product

Fortunately it's an internal tool that I knocked up and have complete control over, so I might even try the alpha out, I'm gung-ho like that at times.

Many thanks for your detailed responses - I will take your suggestions on board and see what I come up with.

I'm certainly in no rush for a released fix, and you're certainly under no obligation to get anything out - I have workarounds, and with your advice above, I have potentially better workarounds 😉

---

And to reply to you PS... unfortunately because it's an internal tool (read - non money-generating), and which arguably doesn't even need fancy progress bars, and could even be a service with no GUI at all... I think I'd have difficulty talking my company into parting with money

HOWEVER: I do understand the importance supporting open source projects, be that through rolling up one's sleeves and getting involved in the code / community (which is exactly how I accidentally became a member on the Pi-hole project 6 years ago....) , and/or chucking money at them - if you'd be willing to enable one-time custom amounts in your sponsorship dashboard - I would be more than happy to personally send you a one off donation

And regards the Gitter conversation:

the .net community doesn't give back ... at all...like ...basically zero.

I entirely understand this frustration, and is probably not exclusive to the .net community, although I'm not involved enough in OSS .net stuff to know one way or the other - but I certainly see it a lot in OSS, usually in the form of:

• Demands for new features as though you owe it to them
• Non constructive bug reports with the expectation that you can read minds
• Claims that they could make something better. In rust. Or Typescript. Or [insert language of the month here] (I always invite these people to go ahead...)

[PromoFaux]

I know it's obvious, but just mentioning it in case for some reason you thought you'd have to make you code all ugly with lots of try { ...Foo.Refresh().. } catch { ...} calls everywhere?

Also very helpful. I'm mostly self-taught in C#, so I'm never surprised when I learn new ways of doing things that are better!

[goblinfactory]

re: one off donation;
I've enabled buy me a coffee. :D

If I had my windows machine up, I'd have committed the fixes instead of typing a lengthy response; would have taken just as long! lol!

Really glad to finally get someone else working hard in OS actually using my lib; or ...using it, and providing constructive interactions ...of any kind :D

[PromoFaux]

💸☕

[goblinfactory]

Got my windows machine finally setup so that I can take a look at publishing a new release with some fixes;
These are queued up alpha changes that will be included, plus the fixes described above;

## [7.0.0.7-alpha/highspeedwriter-only-refresh-dirty-regions]

- busy ... test only refreshing dirty region of screen.
- do perf test writing lines at a time to screen, including scrolling.
- dropped, currently already over 200 frames per second, increased complexity not justified for no visible improvement in UX experience.

## [7.0.0.7-alpha/hsw-perf-faster-borders]

### Improved

- improvement in HighspeedWriter performance 
  - tested rendering full screen (90x30 window) splitWindow left and right. (from `5.46fps`, `183ms` per screen render, to `8.26fps`, `121ms` per request.) Test run 200 iterations.
  - tested rendering updates in split window (90x30 window) with writeLine and scrolling. No changes needed (Currently `289.92fps`, `3ms` per `WriteLine` operation including scrolling and rendering..) Test run 2000 iterations.
  - tested rendering updates in split window (140x60 window) with writeLine and scrolling. No changes needed (Currently `78.18fps`, `13ms` per `WriteLine` operation including scrolling and rendering..) Test run 2000 iterations.

- improvement in standard console rendering, performance tested rendering full screen (90x30 window) splitWindow left and right. (from `1.08fps`, `927ms` per screen render, to `1.71fps`, `602ms` per request.) Test run 40 iterations.

### Removed

- removed cursorVisible. (to make konsole easier to make multiplatform)

## [7.0.0.3-alpha]

### Added

- SplitRows, SplitColumns now supports multiple wildcards per split layout, `console.SplitColumns(new Split(10, "left"), new Split("wild1"), new Split(20, "middle"), new Split("wild2"), new Split(20,"right"));`
- Split windows without border. (added more extensions and tests for splitting.)
- Split adds extra row to bottom of uneven splits to fit parent.
- faster way to create an even number split windows `var rows = con.SplitRows("top", "middle", "bottom");` even split across new windows.

## [7.0.0.1-alpha]

### Added

- massive improvements in stability and loads more concurrency tests, handling lots of edge cases.
- ClipScrolling, ClipWrapping.
- new class `WriteResults` returns the result of writing to a buffer row, including any overflow.
- new peek commands `IPeek` interface to tell what's at a screen location, added to `Window` and `MockConsole`.
  - `Row Peek(int sx, int sy, int width);`
  - `Cell Peek(int sx, int sy);`
  - `Row[] Peek(ConsoleRegion region);`
- `Cell` is now public.
  - `Cell` now has `Colors` property to read the foreground and background at once.
- `Row` is now public.
- new Methods to `IConsole`
  - `void Write(Colors colors, string text);` 
- new method to `IWrite`
  - `void WriteLine(Colors colors, string text);` 
- New controls `ListView` and `DirectoryListView`
- new default colors ..see `Colors.cs`
- new Theming throughout, `IConsole` implements `ITheme` meaning that controls, like `ListView` inherit their theme from their parent window.
- new defaultTheme and colors
  - gray on black
  - white on black
  - black on white
  - white on blue
  - white on darkblue
  - blue on white
  - darkblue on white

### Fixed

- #57 Console.Clear() should not reset the current colors of the window. 
## [6.3.0-alpha]

###  Added

- Ability to create Progress bars without any text. 
  * New progressbar constructor `new ProgressBar()` creates a new `no text ProgressBar, with char `#` and max of 100.
    & New enum value, `PbStyle.BarOnly`
- `IProgressBar` has additional properties
  * `int Current { get; }` property. 
  * `ConsoleColor TextColor { get; set;}`
  * `ConsoleColor BarColor { get; set;}`
- new type 'CharBar' - for ad-hoc bars at (x,y) location, with fixed width, without any text. e.g, quick indicators, or LED effects.

### Minor breaking behavior.

Minor breaking behavior are cosmetic only and wont break an application.

- Default ProgressBar with no values, o

[goblinfactory]

the reason I left these as alpha was that I have been meaning to rewrite the documentation for Konsole to make it much easier to use for a first time user, with a sequence of examples with the most common use cases; and have a longer detailed set of documentation for each feature. writing good documentation takes time so that was postponed until I finished all refactoring.

[goblinfactory]

@PromoFaux Hi Adam
I'm not going to be able to bring out a quick fix as I had hoped; mostly because a patch would then involve me having to merge the patch back into long sequence of branches, and releasing the latest work (most recent branch) involves updating a lot of documentation. My estimate for this is at least 2 or 3 days work; cleaning up where I last left the project, and producting documentation.

My recommedation is to still the latest stable v6 release; and write your own custom "SafeRefresh" extension method as I've outlined above.

I'll ping you when I carve out some time to work more on Konsole and have an update with a change for that included.
(rather than cause exception's to be caught on every refresh, I'll probably add both an exception catch as well as the obvious negative value checks, that will avoid an expensive runtime exception catch.

Thanks for your coffee support, and for using Konsole.
let me know if there's anything i can help with.... I've designed Konsole so that it can easily be extended for custom purposes via the interface using extension methods. I should document examples of that ..mmm...(noted!).
:D

cheers,
Alan

[PromoFaux]

Hey, no worries at all, and certainly no rush - the issues I've seen have been edge-case at best

My current hypothesis is that it is something to do with the window being resized when someone connects with a client with a much smaller resolution than normal (i.e RDP on a phone)

This turned out to be the case - it was always the same person reporting the crash, and it turned out they were connecting from RDP on their phone sometimes, which was causing a window resize to make it fit in the tiny resolution. So for now I've just said "STOP IT" 😏

[goblinfactory]

hahah...ok.
If you do want to fix it, so that you dont need to tell your customer to stop; here's the code; Also, in case anyone else is reading this thread and wants a quick work around

    public static class PBRrefresher
    {
        public static void SafeRefresh(this ProgressBar pb, int current, string item)
        {
            try
            {
                pb.Refresh(current, item);
            }
            catch { }
        }

        public static void SafeNext(this ProgressBar pb, string item)
        {
            try
            {
                pb.Next(item);
            }
            catch { }
        }
    }

then use exactly the same as before but just change all your calls to .Refresh(...), to .SafeRefresh(...), and .Next() to .SafeNext(..)

[goblinfactory]

or if that wording feels wrong; like admitting we've written unsafe code; name it ....VeryVerySafe(...) or .EvenSaferRefresh(...)