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

telemetry breaks sui move test (also it would be nice if it were feature flagged) #17426

Closed
skaunov opened this issue Apr 30, 2024 · 2 comments · Fixed by #17484
Closed

telemetry breaks sui move test (also it would be nice if it were feature flagged) #17426

skaunov opened this issue Apr 30, 2024 · 2 comments · Fixed by #17484
Assignees
Labels
cli Command line tools

Comments

@skaunov
Copy link

skaunov commented Apr 30, 2024

Steps to Reproduce Issue

Fill this in with the concrete steps needed to reproduce the bug. When providing code in the reproduction steps, use the smallest snippet of code that demonstrates the issue, removing any extraneous details.

e.g.

  1. Follow https://docs.sui.io/guides/developer/first-app/build-test#sui-specific-testing.
  2. Run sui move test in KVM.

Expected Result

As per the guide describes it.

Actual Result

sui@pop-os:~/my_first_package$ sui move test
INCLUDING DEPENDENCY Sui
INCLUDING DEPENDENCY MoveStdlib
BUILDING my_first_package
thread 'main' panicked at external-crates/move/crates/move-compiler/src/sui_mode/id_leak.rs:158:56:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2024-04-30T21:36:00.039232Z ERROR telemetry_subscribers: panicked at external-crates/move/crates/move-compiler/src/sui_mode/id_leak.rs:158:56:
called `Option::unwrap()` on a `None` value panic.file="external-crates/move/crates/move-compiler/src/sui_mode/id_leak.rs" panic.line=158 panic.column=56
Aborted (core dumped)

System Information

  • OS: Pop OS (current version, upgraded)
  • Compiler:
sui@pop-os:~/my_first_package$ sui -V
sui 1.24.0-92ba3bc7a

Couple of previous versions also errored on "telemetry".

@stefan-mysten stefan-mysten added the cli Command line tools label Apr 30, 2024
@amnn
Copy link
Member

amnn commented May 1, 2024

Thanks for the report @skaunov -- the issue here is not with telemetry, it's a panic in the compiler (and telemetry is logging that panic):

called `Option::unwrap()` on a `None` value panic.file="external-crates/move/crates/move-compiler/src/sui_mode/id_leak.rs" panic.line=158 panic.column=56

The panicking line is here:

let abilities = self.declared_abilities.get(s).unwrap();

@skaunov, if you have it to hand, can you share the Move code that you sent to the compiler to get it to panic?

@skaunov
Copy link
Author

skaunov commented May 1, 2024 via email

tnowacki added a commit that referenced this issue May 3, 2024
## Description 

- Fixes #17426
- The id-leak assumed pack visibility was a blocking error 

## Test plan 

- Added a test 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [X] CLI: Fixed a bug where the Move compiler could panic when
instantiating an object outside of its defining module.
- [ ] Rust SDK:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Command line tools
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants