-
Notifications
You must be signed in to change notification settings - Fork 167
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
Include the constant name for assert error messages #1289
Comments
I think this would be better solved by having proper source-level debug information associated with a program, which I think should be a pretty high priority in general because of how critical it is to user experience (and for us, in terms of efficient troubleshooting/debugging). To implement this as requested, we'd need to implement the same kind of auxiliary structure as would be needed for debug info anyway, so I see it as essentially the same amount of effort involved. Source-level debug info also works equally well for code generated by the compiler. The vast majority of our users are more than likely going to be working in a higher-level language, not MASM, so we want to be able to emit errors/diagnostics that speak in terms the user understands. By using source spans, we can show the snippet of code associated with a particular error in whatever language it was actually written (assuming a program was distributed with debug info), whereas an error specific to MASM/MAST is not going to be particularly helpful. I've been wanting to write up a proposal for debug info, but haven't been able to get to it, but this is a reminder to do that sooner rather than later I think. |
I think having error messages as described in the original issue would be helpful, but I would like to see it done somewhat differently. I think the current error approach has mainly two issues:
I think source level debug information would be very valuable for the developer experience when debugging code, be it MASM or a higher-level language. But I think it would primarily help with adding context (e.g. I'm thinking about a stacktrace here with human-readable procedure names). What remains is that a developer still needs to lookup the error code themselves and that by itself can be a frustrating experience ("where do I start looking for this?"). I had exactly that experience when I was using another Smart Contract VM that only gave error codes and it was really frustrating having to lookup the error code every time it was failing. Once the error code is found and mapped to its human-readable error, it must still be interpreted, likely at the source itself to understand what condition failed the assertion. Consider this procedure in the miden-base transaction kernel:
This example is one of a typical procedure pretty deep in the call stack and it is likely called many times during the kernel execution. If one now reads "Provided storage slot index is out of bounds" as the error message of a program execution (which we have as a special case), it is not very helpful for debugging. Source-level debug information with a stacktrace would be very valuable here to get an idea at which point the error occurred and which of the procedures the developer was writing was being executed at that time. But I still think that two things could improve the developer experience by a lot at this point:
As a side note, the kernel errors are special in the sense that we have some compile-time tools to create a map of error code -> error message, which is the comment above the error code. This is only a solution for the transaction kernel and is not a generic solution for third party libraries. Native error messages I think point 1 implies that the error message should be defined together with the assertion and needs MASM-level support at the language level. I'm thinking of something like this:
I imagine this would be debug-only information, could perhaps be stored as a decorator and would therefore not affect execution. It is most useful and needed at development time so it being debug-only seems totally fine. Formatting errors
We might then want some additional format specifiers like hex-format, e.g. A really good error message here would really be something like:
This would give the developer the information that they were trying to access the third slot (index 2) somewhere but their account component might only have been defined with two slots in total. One issue is that formatting this message requires having those values on the stack and, as the function is written, they are not. I imagine this would be a fairly frequent problem. So we would either produce subpar messages or we would have to change the program to leave some items on the stack in case the assertion fails so that they are available for formatting. Changing the program might not be desirable as it would affect performance I imagine. Perhaps there are other solutions to this that I can't think about. But I think the general points stand:
Overall I think this would complement source-level debug information very nicely. Knowing which line failed is very helpful but also having a parameterized error message would go quite far in helping the developer understand what the problem is. Quite frequently, a good error message can give the right hint and save time without having to set breakpoints, insert trace statements and start a full debug session. Do you have any thoughts on this @bitwalker? |
@PhilippGackstatter I agree that having a way to convert error codes to meaningful descriptions of what they mean is hugely helpful! I think one example of doing this well, is with Rust itself. The compiler currently provides source-level debugging capabilities like I described in my previous comment, via the
That has vastly improved the debugging experience IMO. There is still some major room for improvement though (and naturally, the UI itself would benefit from better UX). I've found the following to be the biggest areas that I would personally like to see us focus on:
Regarding formatting, I have a few thoughts on this: I think the most sensible solution is to make use of the existing As a toy example, consider One thing I'd like to try and get us in the mindset of, is that Miden Assembly being the primary way to interface with Miden should be considered a temporary, short-term state of affairs. My opinion is that our priority should be to add capabilities to the tooling and infrastructure of libraries/packages and runtime APIs for introspecting the state of programs and why they fail. Adding surface syntax to Miden Assembly should really be a secondary priority. As an example of why using MASM as a starting point produces less-than-ideal results - let's say we introduce the error formatting infrastructure to the syntax. This is not going to be particularly useful to high-level languages being compiled to Miden Assembly. Rust, for example, provides panic formatting, which conceptually seems like it could take advantage of that functionality, but there is significant distance in semantics between Rust source-level panics and MASM assertions. Furthermore, we are compiling Rust to WebAssembly, which is a The debugger provides me with all the tools I need to examine values on the operand stack or in memory, and format them how I want them. What I don't have, and really want, is an explanation for compiler/assembler-generated assertions that may not have any associated source code, but are nevertheless associated with some invariant being enforced (think arithmetic overflow, bounds checks, etc.). We could specify error codes used by the compiler/assembler for such things, and in conjunction with user-defined error codes, be able to provide better contextual information about why a particular assertion was hit. I think it is questionable whether formatting facilities at the MASM level are really worth the complexity cost though, considering that you can just use the debugger to poke at the values yourself. Anyway, I think that about summarizes my take on the current state of affairs and where the biggest wins are in the near future. I am basing all of this on the debugger in the compiler though, and the state of things without that is much worse. It is a priority (to me) to get the |
Totally agree. We could change how the "stack helper columns" work so that they also track the "true depth" of the stack (as opposed to only tracking when it exceeds 16). Related: #1537 |
We are using
assert.err=<code>
to give context for a given error.<code>
may be a integer or a constant, a failing assert likeassert.err=ERR_INVALID_NOTE_TYPE
will then result in an error similar to:There are a few issues with the above:
131140
in the code base will give no results0x20044
may not give a hit, for the example above the value was defined as0x00020044
The above makes it unnecessarily convoluted to find the corresponding error. A simple fix for this is to add the string
<code>
in the error message. So the error above would be:If defined with a hex directly, i.e.
assert.err=0x00020044
:And if defined with a number, i.e.
assert.err=131140
:The text was updated successfully, but these errors were encountered: