Crash when piping to a pager like less, bat

[bb]

jql crashes for me when piping the output to a pager like less or bat and the output is not fully read (because it's longer than the viewport). I'm running version 2.9.3. installed via brew on macos (latest version).

This works as it only provides very few lines of output for my example file:

RUST_BACKTRACE=1 jql '.!'  tmp.json | less

However, when outputting a bit more, it crashes when I quit the pager without scrolling to the end:

RUST_BACKTRACE=1 jql '.'  tmp.json | less
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1021:9
stack backtrace:
   0: _rust_begin_unwind
   1: std::panicking::begin_panic_fmt
   2: jql::render_output
   3: <async_std::task::builder::SupportTaskLocals<F> as core::future::future::Future>::poll
   4: jql::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
[1]    60860 abort      RUST_BACKTRACE=1 jql '.' tmp.json |
       60861 done       less

When I scroll the less to the end (e.g. by pressing G key) or when I pipe to cat which also reads everything, there's no issue.

Please let me know when you need more information.

[yamafaktory]

Hi @bb and thanks for reporting the issue.
So to recap, it works with cat but not with less, right?
Can you provide the failing JSON file?
I'll have a look at it anyway.

[bb]

Yes. Works with any consumer which reads to the end, e.g. cat. Fails with those who only read part of the output, e.g. less (assuming output is longer than screen height. Otherwise less reads all of it and it works).

[bb]

Just noticed I only answered one of the two question. I worked with geojson files from https://osm-boundaries.com/Map. But the actual file does not matter, as long as it's longer than the screen size and bigger than some internal buffer.

The geojson files need OSM login, but there here are two examples of (non-geojson) json data:

This fails:
curl https://osm-boundaries.com/ajax.php\?cmd\=GetTreeContent\&db\=osm20201228\&rootId\=-2202162 | jql "." | less (If you press q in less WITHOUT scrolling down. File size is 2700k.)

While this works:
curl https://osm-boundaries.com/ajax.php\?cmd\=GetTreeContent\&db\=osm20201228\&rootId\=%23 | jql "." | less (16k).

[yamafaktory]

Yeah, the problem is that jql reads the whole piped content in memory and expect it to be valid JSON. less - and by extension bat (which is using less by default https://github.com/sharkdp/bat#automatic-paging) - is paging the output which is thus incomplete JSON.
I might have to look at https://docs.rs/serde_json/1.0.62/serde_json/struct.StreamDeserializer.html.
Will keep you posted.

[yamafaktory]

@bb sorry for the late reply!

I was checking again and indeed jql is already using what I mentioned in the previous message. It seems to be a Rust issue in the end rust-lang/rust#46016.

They mentioned multiple workarounds including this one used by ripgrep https://github.com/BurntSushi/ripgrep/pull/586/files.

Since it's using unsafe, I'd prefer to avoid it... but it might be the only solution currently available.

[bb]

@yamafaktory thanks for following up. No need to worry or feel sorry. Honestly, I finished the stuff I was using jql for and don't need it now or anytime soon. So it's absolutely not critical nor urgent for me...

My workaround was to press Shift-G in less so it scrolls to the end and thus prevents the crash.

[yerke]

@yamafaktory You might want to try the approach similar to crev-dev/cargo-crev@3f3dedf (as discussed in crev-dev/cargo-crev#287). No unsafe there.

[yamafaktory]

@yerke I guess this is what I mentioned a while ago here #86 (comment)

[yamafaktory]

@yerke nevermind this was just one of the links, I see what you mean (commit).

[yamafaktory]

I'm going to release a new version soon, thanks @yerke!

Looking good now: https://asciinema.org/a/465294.

[yamafaktory]

@bb A new version has been released. Don't hesitate to re-open the issue if the fix doesn't work has expected (works for me locally and in the CI).