Skip to content
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

Fix pallet error spam when there is really just one simple error #213

Open
Tracked by #264
sam0x17 opened this issue Feb 21, 2023 · 5 comments
Open
Tracked by #264

Fix pallet error spam when there is really just one simple error #213

sam0x17 opened this issue Feb 21, 2023 · 5 comments
Assignees
Labels
D3-involved Can be fixed by an expert coder with good knowledge of the codebase. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@sam0x17
Copy link
Contributor

sam0x17 commented Feb 21, 2023

Created from this comment #264

Currently, if your pallet has a syntax error, you get an error for it, and then a gzillion other errors because the corrupt/incomplete pallet code is still being treated as Rust code. Ideally, we'd tell the compiler to stop immediately.

This is largely due to missing trait impls or structs during pallet expansion resulting from some error that prevented some part of the expansion from running. There are a lot of situations where this can happen, so the surface area for fixing this issue could be rather large, however the approach should be to proactively detect that something is missing and issue a compile error there, before pallet expansion begins.

Generally speaking, we want to make sure that the "validation" phase of all of our pallet macros happens during parsing, and that avoid issuing errors during expansion

@bkchr
Copy link
Member

bkchr commented Feb 21, 2023

I also thought about this already multiple times. If we introduce some validation phase, which we generally already have as the parsing phase or whatever we do, we should always expand. Maybe if we detect issues we expand with some "best effort". Otherwise we would still see a lot of unrelated errors.

But yeah, in general that is probably something that requires some tinkering to find the best way to handle certain errors.

@sam0x17
Copy link
Contributor Author

sam0x17 commented Feb 21, 2023

Yeah, I think the key is to catch these types of errors and issue a compiler error with an appropriate span before any expansion has started, but will have to try it to know for sure

Doing "best effort" stuff might actually be causing this in some cases where because something is missing beacuse something else failed, we get 100 errors

@bkchr
Copy link
Member

bkchr commented Feb 21, 2023

I don't think that we currently generate types on best effort. Afaik we bubble up the error to the top and then don't generate anything.

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added T1-FRAME This PR/Issue is related to core FRAME, the framework. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed T1-runtime labels Aug 25, 2023
@kianenigma kianenigma added D3-involved Can be fixed by an expert coder with good knowledge of the codebase. and removed D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Feb 6, 2024
@gupnik gupnik self-assigned this Mar 11, 2024
@Polkadot-Forum
Copy link

@Polkadot-Forum
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D3-involved Can be fixed by an expert coder with good knowledge of the codebase. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Draft
Status: Backlog
Development

No branches or pull requests

6 participants