Skip to content

test catching a class of exception; Should catch not all exceptions, but only the required class of exception?#1290

Merged
Hecate2 merged 2 commits intoneo-project:masterfrom
Hecate2:test-catch-class
Feb 6, 2025
Merged

test catching a class of exception; Should catch not all exceptions, but only the required class of exception?#1290
Hecate2 merged 2 commits intoneo-project:masterfrom
Hecate2:test-catch-class

Conversation

@Hecate2
Copy link
Contributor

@Hecate2 Hecate2 commented Feb 5, 2025

I am confused whether this is a bug or a feature:

using Neo.SmartContract.Framework;
namespace Neo.Compiler.CSharp.TestContracts
{
    public class Contract_TryCatch : SmartContract.Framework.SmartContract
    {
        public static string catchExceptionType()
        {
            string result = "NoException";
            try { throw new System.Exception(); }
            catch (UncatchableException e) { result += e; }
            return result;
        }
    }
}
# Method Start Neo.Compiler.CSharp.TestContracts.Contract_TryCatch.catchExceptionType()
00 INITSLOT 02-00 # 2 local variables, 0 arguments
# Code test-compiler.cs line 21: ""NoException""
03 PUSHDATA1 4E-6F-45-78-63-65-70-74-69-6F-6E # as text: "NoException"
# Code test-compiler.cs line 21: "result = "NoException""
16 STLOC0
17 TRY 0F-00 # catch pos: 32 (offset: 15), no finally block
# Code test-compiler.cs line 22: "throw new System.Exception();"
20 PUSHDATA1 65-78-63-65-70-74-69-6F-6E # as text: "exception"
# Code test-compiler.cs line 22: "throw new System.Exception();"
31 THROW
# Code test-compiler.cs line 23: "catch"
32 STLOC1
# Code test-compiler.cs line 23: "result += e"
33 LDLOC0
# Code test-compiler.cs line 23: "e"
34 LDLOC1
# Code test-compiler.cs line 23: "result += e"
35 CAT
# Code test-compiler.cs line 23: "result += e"
# Code test-compiler.cs line 23: "result += e"
36 CONVERT 28 # ByteString type
# Code test-compiler.cs line 23: "result += e"
# Code test-compiler.cs line 23: "result += e"
38 STLOC0
39 ENDTRY 02 # pos: 41 (offset: 2)
# Code test-compiler.cs line 24: "result"
# Code test-compiler.cs line 24: "return result;"
41 LDLOC0
# Method End Neo.Compiler.CSharp.TestContracts.Contract_TryCatch.catchExceptionType()
# Code test-compiler.cs line 25: "}"
42 RET

It catches any exception and returns NoExceptionexception. Maybe we need to judge the exeption type, or simply allow catch (System.Exception e) only.

@shargon
Copy link
Member

shargon commented Feb 5, 2025

UncatchableException can't be catched

@Hecate2
Copy link
Contributor Author

Hecate2 commented Feb 5, 2025

UncatchableException can't be catched

When I write catch (UncatchableException e) but throw a System.Exception, I think the exception should not be caught. But the System.Exception is actually caught.

@shargon
Copy link
Member

shargon commented Feb 5, 2025

We don't have exceptions kinds in the vm... @Jim8y proposed in the past

@nan01ab
Copy link
Contributor

nan01ab commented Feb 5, 2025

It is weird, but it's a feature.

@Hecate2 Hecate2 merged commit e2b81ad into neo-project:master Feb 6, 2025
3 checks passed
shargon
shargon reviewed Feb 6, 2025
View reviewed changes
Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LgTM too

@Hecate2 Hecate2 mentioned this pull request Feb 7, 2025
Merged
Jim8y pushed a commit that referenced this pull request Aug 3, 2025
@Hecate2
test catching a class of exception (#1290)
e653616
Jim8y pushed a commit that referenced this pull request Aug 18, 2025
@Hecate2
test catching a class of exception (#1290)
4fa175c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shargon shargon shargon left review comments

@Jim8y Jim8y Jim8y approved these changes

+1 more reviewer

@nan01ab nan01ab nan01ab approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Hecate2 @shargon @nan01ab @Jim8y