Missing pool name causes leak of opaque error into stacked bar area (change at Zed 68f35c9)

[philrz]

tl;dr

When starting a query session without any specific pool, the changes associated with brimdata/super#5121 have caused an error to start appearing in the stacked bar chart area that's related to one of the queries the app issues behind the scenes and hence is likely to be confusing to a user.

Details

Repro is with Zui commit 4354c02. This issue is regarding the behavior when a user clicks the + and selects New Query Session which starts a new tab without a pool pin and hence sees no initial Zed query or results.

Consider the following video taken with GA Zui v1.7.0 as a baseline. The stacked bar chart area is populated with an error undefined: pool not found and below that the statement No Pool Specified and a button to Create Pool. The error message in the stacked bar chart area is not ideal, but the overall UX seems to reflect the divergence from the more common "happy path" where a user has clicked a Query Pool from the pool summary page.

Zui-v1.7.0.mp4

Now contrast this with the following video taken with Zui commit 4354c02. Repeating the same sequence of clicks, we see two significant changes:

1. The error in the stacked bar area now says the following:

undefined: pool not found at line 1, column 6: from undefined | min(ts), max(ts) ~~~~~~~~~

This query with the min(ts), max(ts) appended is issued by the app to help populate the stack bar chart. But since it wasn't issued directly by the user, we generally would not want to show them syntax errors related to the appended portion.

2. The lower portion of the screen now shows Error no pool name given rather than the previous button to help them select a pool.

Repro.mp4

I narrowed this down to having started happening when Zui's pointer for the Zed dependency advanced to 68f35c9, which is associated with the changes in brimdata/super#5121.

I studied the client/server traffic over the network in the attached pcaps zui-1.7.0.pcapng.gz and zui-4354c02.pcapng.gz. I noticed that in the baseline Zui v1.7.0 case, everything stops after the client issues the {"query":"*\n | { i: count(), v: this}\n | i > 0\n | head 500\n | yield v"} and gets back {"type":"Error","kind":"invalid operation","error":"pool name missing"}. Meanwhile, in the 4354c02 case that reflects the current behavior, the client issues that same query and gets back {"type":"Error","kind":"invalid operation","error":"no pool name given"} (so, a slightly different error, but similar essence) and then marches on to still issue the query {"query":"from undefined | min(ts), max(ts)"}. I suppose this is all in the spirit of what's described in brimdata/super#5121, i.e.:

This commit also changes how semantic errors are collected in that the
semantic analyzer no longer exits as soon as the first error is found
and instead goes on the analyze the rest of the AST, reporting all
semantic errors in the AST.

However, this does seem like a situation when that behavior is perhaps undesirable, though I'm not sure what to do about it.

I also assume there's probably other spots in the app where behind-the-scenes queries are being issued and we'd want to be mindful of making sure errors are surfaced appropriately, so we'll want to put some thought into that even as we address this one case.

[philrz]

Verified in Zui commit bfbb55e.

I'm pleased to report that the changes in #3076 have fixed the majority of the problem described above. As shown in the video below, now after clicking the + and selecting New Query Session, the opaque error message from the behind-the-scenes query is no longer shown in the stacked bar chart area and the lower portion of the screen once again shows No Pool Specified and the Create Pool button.

Verify.mp4

One change still present that makes it different from the way it was before is that the error in the stacked bar chart area now shows No date range found with 'ts' instead of the prior undefined: pool not found. As opaque errors go, maybe this is slightly worse than what was there before, but it feels like splitting hairs since neither is ideal. I've opened #3081 with the goal to surface something else, but I'm closing this issue since the major problems have been addressed.

Thanks @mattnibs!