-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error message from zq sometimes not surfaced #3021
Comments
As an idea to further debug this, I've been testing on the zui-3021-debug branch where I've created a wrapper script around the calls to
tl;drThe mere presence of the wrapper script seems to make the test pass consistently. Detailshttps://github.com/brimdata/zui/actions/runs/8090373897 was a baseline run where I'd just created the branch based off a recent tip of https://github.com/brimdata/zui/actions/runs/8102744996 was the run that started from that baseline and also included all changes shown here to introduce the wrapper script and have it called in lieu of directly calling I'm not sure what conclusions to draw from that result. 🤔 |
tl;dr
When Zui calls on
zq
in a way that's expected to fail (such as in Preview & Load when trying to read a file that is not of a format that Zed can read via auto-detect) the expected error message sometimes is not surfaced.Details
Repro is with Zui commit 57afe04.
The repro in the video below is taken on a Linux VM run in VirtualBox. The file that's being opened in Zui is the same soccer-ball.png that's opened by this e2e test which has been described as "flaky":
zui/packages/zui-player/tests/pool-loads.spec.ts
Lines 42 to 46 in 57afe04
In the video I begin the process to load the file in Preview & Load several times. While the auto-detect failure is shown in the app most times, you can see on both the second and final attempts that no error message is shown.
Repro.mp4
What led me to repro this manually is that I'd been studying that "flaky test" shown above via a branch where I ran just that test repeatedly in CI and found it failed 24 times out of 100 in a manner similar to what I showed in my manual repro video.
Based on what's shown here I suspect this may share a root cause with a separate Jest test that's been known to fail occasionally in CI that's been on my list to study:
zui/packages/zed-node/src/zq.test.ts
Lines 106 to 115 in 57afe04
On the same Linux VM where I did the repro/video above I triggered a failure via this loop:
To get to a quicker repro, I deleted all the other tests so just this one "zq with a bad zed" test would run, and I also first started a
cat /dev/random > /dev/null &
for every core on my VM since keeping the system under load seems to force the repro quicker (presumably because heavy load aggravates timing issues). With this approach I got 3 repros out of 218 runs, with this being an example of a failure:The rejection of the promise appears to be a prerequisite for seeing this error, which leads me to guess that in my manual repro in Preview & Load the lack of visible error message is similarly due to the promise resolving like we see here.
Conclusion
When I reviewed the latter test failure with @jameskerr in the past he acknowledged that the code in zq.ts that deals with the callouts to
zq
is a little complex and probably tricky to debug. But now that we've confirmed a user-facing bug is reproducible and it's not just a "flaky test", it seems worthwhile to invest some cycles to look closer at this.The text was updated successfully, but these errors were encountered: