someodd / waffle

Haskell Gopher Protocol TUI Client 🧇
https://www.someodd.zip/showcase/waffle/
GNU General Public License v3.0
21 stars 1 forks source link

crash/exit on trying to access bookmarks #129

Closed inkstain closed 4 years ago

inkstain commented 4 years ago

Hi I just started using this yesterday (seems great thus far!) but while I am able to add bookmarks okay, as soon as I try to retrieve the list via Ctrl+b the terminal window just disappears. I have tried deleting the bookmarks.ini and creating a new one, and testing after just one bookmark but the same occurs. Visually the bookmarks.ini contents seem fine

I am running a clean Fedora 32 VM on Qubes OS with the GHC 8.10.1 with cabal 3.2.0.0 (I had some concerns about running the newer version of Haskell and cabal on my other vms so set aside a new otherwise untouched one.)

Please let me know if you need any additional info.

thx!

inkstain commented 4 years ago

Just an additional observation:

Originally I was using within gnome-terminal which is the default, but just in case, I tried in sakura and while waffle still crashes, the terminal window survives with the following error

waffle: Parse bookmark error: no gophertype
CallStack (from HasCallStack):
  error, called at src/BrickApp/ModeAction/Bookmarks.hs:60:24 in waffle-0.13.0.0-inplace:BrickApp.ModeAction.Bookmarks

thx

hyperrealgopher commented 4 years ago

Welcome! Thank you for showing interest in this project and taking the time to file a bug report.

Could you please post an ini file which crashes the bookmark viewer (ctrl + b)? I saw waffle: Parse bookmark error: no gophertype pop up a few times. I think it's related to bookmark names having something that's unparseable for section names in the parser I use for INIs.

On a side note, have you heard of ghcup-hs? I thought of it immediately when you were talking about concerns related to newest GHC and Cabal.

inkstain commented 4 years ago

Here is what I currently have in ~/.config/waffle/bookmarks.ini

[Circumlunar Republic (republic.circumlunar.space:70/1/)]
host: republic.circumlunar.space
port: 70
resource: 
type: 1

[Floodgap]
host: floodgap.com
port: 70
resource: /
type: 1

[Floodgap (gopher.floodgap.com:70/1/)]
host: gopher.floodgap.com
port: 70
resource: /
type: 1

[SDF Phlogs]
host: sdf.org
port: 70
resource: phlogs/
type: 1

But I have tried nuking it and just creating one entry and it failed. But in trying to reproduce this I find that after removing the bookmarks.ini (so all that is in the config folder is the default open.ini) and launching waffle and checking for bookmarks before adding one I see two of the ones I added previously so maybe it is somehow pulling from different locations and if there is a double it errors out? Where else might it be pulling from?

When I gracefully closed out of waffle I checked and it recreated the bookmarks.ini and populated it with the two I saw within the program (that I did not manually add today)

The "auto-generated" ini shows:

[SDF Phlogs]
host=sdf.org
resource=phlogs/
type=1
port=70

[Floodgap]
host=floodgap.com
resource=/
type=1
port=70

Thanks for the heads up on the ghcups-hs project, will need to check that out.

inkstain commented 4 years ago

sorry, inadvertently clicked 'close' :)

hyperrealgopher commented 4 years ago

No worries and thanks again.

Here is the default bookmarks.ini from the data folder, which is used to create your ~/.config/waffle/bookmarks.ini: https://github.com/hyperrealgopher/waffle/blob/master/data/bookmarks.ini

I'm not sure if the above answers any questions...

I copied your INI and it crashes just the same for me. I'll look into which [section name] is doing this and why and release a patch probably within the day.

hyperrealgopher commented 4 years ago

I think there's something seriously wrong with the parser I'm using.

So apparently the problem is this SectionSpec:

[Circumlunar Republic (republic.circumlunar.space:70/1/)]

... but I can change it to this:

[Foo Republic (republic.circumlunar.space:70/1/)]

... and it works. Is the same true for you? I suspect so. I'm not sure if there's some weird max character limit for the SectionSpec in the library I'm using.

hyperrealgopher commented 4 years ago

... wait apparently even if I replace [Circumlunar Republic (republic.circumlunar.space:70/1/)] with [Oneverylongword Republic (republic.circumlunar.space:70/1/)] it still works... it only seems to crash with Circumlunar... I have no idea what the parser is doing here...

hyperrealgopher commented 4 years ago

[Circumlunar] even crashes the parser! So does [Circumluner], [Circumlener], [Circemlener], [Cercemlener].

Interestingly, [Bircumlunar] crashes, but [Zircumlunar] does not, nor does [circumlunar].

hyperrealgopher commented 4 years ago

I'm guessing this has to be a major flaw in the parser... I'm going to email the author of Data.ConfigFile in addition to investigating deeper into their code.

inkstain commented 4 years ago

Sorry I missed the party! I went to sleep early as there is a big day today. Looks like you got it narrowed down though; as you say it seems an odd sort of error for the parser to "provide". Your default bookmarks explains that other thing I stumbled across. By coincidance, both of those defaults were ones I added manually early on (not knowing there were defaults) so I thought it was reading it from some tmp file from the crashes though I did not see any bookmark data in the tmp files.

Many thanks for looking into this and for all the work you have done on waffle. It is already my favorite among the gopher clients I have tried [Bombadillo, Kristall, Amfora, and some others]

hyperrealgopher commented 4 years ago

Thank you so much, @inkstain! I'm honored that you tried many other Gopher clients and decided you like Waffle the best. It truly gives me a lot of joy to hear that.

Glad to hear me pointing out the default bookmarks.ini cleared any confusion. I figured it'd be something like that. What a coincidence!

It'll take me a while to get to the bottom of fixing this issue (because it is an upstream issue, related to the library I use for INI files). I might just use a different library.

Here's some more weirdness: [C] will crash the parser... Other things:

kiranandcode commented 4 years ago

Posted this on activitypub but I'll repeat here in case you didn't see it.

Are you sure the error is coming from the ConfigFiles library? I was intrigued by this error, so I tried to replicate it locally, but found the ConfigFiles library to work fine - I also looked through their source code and couldn't find any explanation for this particular set of errors.

For reference, I used the following code:

import Data.Either.Utils
import  Data.ConfigFile
import Control.Monad.Error
import Control.Monad.Identity
import Control.Monad.Except

user_bookmarks_file =  "./example.ini" 

unsafe_unwrap cp = 
  let output = runExcept cp in
  case output of
    Left v -> error ("failed " ++ (show v))
    Right (v) ->
      return $ v

load_config_file file = do
  cp <- readfile emptyCP file
  let output = runExcept cp
  unsafe_unwrap cp

append_bookmark cp (section_spec, host, port, resource, item_type) =
  unsafe_unwrap $ do
      cp <- add_section cp section_spec 
      cp <- set cp section_spec "host" host
      cp <- set cp section_spec "port" (show port) 
      cp <- set cp section_spec "resource" resource
      cp <- set cp section_spec "type" (item_type : [])
      pure cp

example_usage = do
  cp <- load_config_file user_bookmarks_file
  cp <- append_bookmark cp ("Apple", "www.applesucks.org", 182, "Apple", 'c')
  cp <- append_bookmark cp ("0", "www.fsf.org", 182, "Apple", 'c')
  cp <- append_bookmark cp ("B", "www.fsf.org", 182, "Apple", 'c')
  cp <- append_bookmark cp ("Bakery", "www.fsf.org", 182, "Apple", 'c')
  cp <- append_bookmark cp ("1", "www.fsf.org", 182, "Apple", 'c')
  cp <- append_bookmark cp ("9", "www.fsf.org", 182, "Apple", 'c')
  pure (to_string cp)

While reading from the following ini file:

[Bapple]
gackle = 10
[Apple1]
opt1 = A
google = 110
[Dog]
dog = "20"
[D]
tmp = "file"

The library worked correctly, and the resulting config file was as expected. (Running on Gnu/Linux).

hyperrealgopher commented 4 years ago

Hello @Gopiandcode! Thank you for reaching out and investigating this issue. I'm not really sure what I could be doing wrong where different [section titles] would result in this behavior. It's interesting that those all work for you.

The error is related to this line #60 in ModeAction/Bookmarks.hs. If I change the section title it will make this error trigger with the section titles I mentioned in my last post.

I honestly was so confused by this I started to use Data.Ini instead.

But now that I investigate it further I think I found the culprit: tail and the order of things... I assumed DEFAULT always came first, but that's not the case! I think I like this Data.Ini library more, but I'll see this through and post an update soon.

Thank you so much for getting in touch with me and making this post!

hyperrealgopher commented 4 years ago

Yes, changing the line to be let sectionOptionsList = filter (\x -> fst x /= "DEFAULT") $ Map.toList $ CF.content cp actually fixes the issue... I feel very foolish. :rofl:

I even read through Data.ConfigFIle's source to be sure and didn't see anything bad, either. Lesson learned--I should be far more explicit!

Update: So I assume this means it's sorted alphabetically!

hyperrealgopher commented 4 years ago

@inkstain this has been fixed in https://github.com/hyperrealgopher/waffle/releases/tag/v0.16.0

Thanks to both @inkstain and @Gopiandcode for all the help!