Panic in Playground shows bad error message

[richb-hanover]

What happened?

When editing a query in the Playground (using 1759a86 from 25 Nov 2023) the query looks OK when pasted in. But editing inv.name in the nested group, the displayed error message is incorrect.

• changing to in.name correctly shows an error
• adding the "v" back doesn't change the error
• adding another "v" displays the correct error
• removing the extraneous "v" doesn't change the displayed error

I made a video of the behavior... https://youtube.com/shorts/uB66piyGihI?feature=share

NB: It helps to reload the Playground before starting the experiment

PRQL input

from inv=invoices
join item=invoice_items (==invoice_id)

group { inv.billing_city } (

  group { item.name } (
    aggregate {
      ct1 = count inv.name,
    }
  )
)

SQL output

N/A

Expected SQL output

N/A

MVCE confirmation

• Minimal example
• New issue

Anything else?

No response

[max-sixty]

This is because the compiler panics here:

pbpaste | cargo run -p prqlc -- compile -

The application panicked (crashed).
Message:  cannot find cid by id=95
Location: prqlc/prql-compiler/src/semantic/lowering.rs:901

So we could split this into a couple of issues — the panic on this query, and the playground showing better error messages for panics.

On the panic display — we have

prql/prqlc/bindings/js/Cargo.toml

Line 34 in 23494d1

console_error_panic_hook = {version = "0.1.7", optional = true}

— but I don't see panics in the console, so possibly it's not working well...

[richb-hanover]

I always wondered what a panic would look like in the Playground... Not much evidence, except the peculiar behavior shown above.

I see this in the browser Console: panicked at 'cannot find cid by id=95', prqlc/prql-compiler/src/semantic/lowering.rs:917:21 - perhaps this string could be shown in the error-pane:

• It would provide clear evidence to the reader that the problem isn't with their query (so they can stop fiddling around...)
• The combination of the PRQL query plus the error string makes a pretty good bug report
• The message could provide the advice that they will need to reload the browser to "get better"

[max-sixty]

I see this in the browser Console: panicked at 'cannot find cid by id=95', prqlc/prql-compiler/src/semantic/lowering.rs:917:21

Ah, great, I missed this. So the console_error_panic_hook is working as intended

perhaps this string could be shown in the error-pane:

I think it's basically not possible to show it in the error-pane, since the thing that would show it has exited. So we should just try and not panic, and any panic is a bug...

Do you want to adjust this issue to specify this as a compiler bug for panicking?

[richb-hanover]

I think it's basically not possible to show it in the error-pane, since the thing that would show it has exited.

Hmmm... I had started speculating on this, and asked ChatGPT "how can I intercept javascript calls to console.error()" and it gave the answer below. (Actually, this is one of two substantially similar answers.)

My thought was, "all we need to do" is drop this code into some main JS file (this is where my knowledge of the programs structure gets really fuzzy). This replacement function then passes the argument (or some portion of it) to code that sets the text in the error-pane (and now I'm madly waving my hands...) and then call the "real" console.error() function.

Or have I missed your point?

const originalError = console.error;

console.error = function(...args) {
    // Your custom logic here before the actual error is logged
    // For example, you can send it to a logging server or modify the error message
    // ...

    // Call the original console.error() function
    originalError.apply(console, args);
};

Updated to call the other function, which is closer to what I intended.

[max-sixty]

I think you might well be right! If we can intercept the panic and put the message in the error pane, that could work well.

(I'm extremely bad at JS, so I won't be the person to do this, and possibly I shouldn't be opining on it. But I would very much welcome a solution, either to this or to getting this to show an error, which isn't even a panic:

prql target:sql.postgres
#

)

[richb-hanover]

I wonder if @vanillajonathan could take a shot at catching the panic message, since he did the Vite.js conversion...

@max-sixty - I won't let you say, "I'm bad at this..." Don't run yourself down :-) I would accept your statement something like, "I'm not comfortable in the JS environment, so I won't be the person to do this..." Thanks as always.

[vanillajonathan]

Okay, the playground is fixed now, but the compiler still needs to be fixed.

[richb-hanover]

Okay, the playground is fixed now, but the compiler still needs to be fixed.

PR #3870 focuses on the compiler panic. I am closing this as suggested by @max-sixty