parsetoml icon indicating copy to clipboard operation
parsetoml copied to clipboard

parsetoml: make TomlError a non-Defect again

Open ee7 opened this issue 3 years ago • 2 comments

We should be able to catch an exception if we parse invalid TOML, and parsetoml raises a TomlError for such a situation. However, commits 668491edaff1 and 7126677392aa changed TomlError from an Exception to a Defect when compiling with Nim 1.4.0 or newer.

From the Nim Manual (here and here) a Defect should not be caught:

The exception tree is defined in the system <system.html>_ module. Every exception inherits from system.Exception. Exceptions that indicate programming bugs inherit from system.Defect (which is a subtype of Exception) and are strictly speaking not catchable as they can also be mapped to an operation that terminates the whole process. If panics are turned into exceptions, these exceptions inherit from Defect.

Exceptions that indicate any other runtime error that can be caught inherit from system.CatchableError (which is a subtype of Exception).

[...] Exceptions inheriting from system.Defect are not tracked with the .raises: [] exception tracking mechanism. This is more consistent with the built-in operations. The following code is valid:

proc mydiv(a, b): int {.raises: [].} =
  a div b # can raise an DivByZeroDefect

Make TomlError catchable again, making it inherit from a CatchableError (in particular, ValueError). This is consistent with e.g. std/parsejson and jsony.


Pinging @PMunch, as the reviewer of https://github.com/NimParsers/parsetoml/pull/42.

ee7 avatar Aug 16 '22 09:08 ee7

Hmm, the tests seems to be failing now as the tests doesn't take these as being catchable into account. You should update the tests so that they don't output anything when an exception is thrown.

Otherwise this is a great change, these exceptions should definitely be catchable.

PMunch avatar Aug 16 '22 09:08 PMunch

Thanks for the quick response.

Are you sure the test failures are related?

On both master and this branch I see the same failures, in the "Go tests" part of CI:

toml-test [decoder/decoder]: using embedded tests: 274 passed, 56 failed,  3 skipped
     Error: Exception raised during nimble script execution

Furthermore, neither https://github.com/NimParsers/parsetoml/commit/668491edaff1abd5ec31c3f294e1a918d179f13f nor https://github.com/NimParsers/parsetoml/commit/7126677392aa0c4cc9d16bd79d312ed57f15f8f1 modified any test, so I don't see what to do.

ee7 avatar Aug 16 '22 09:08 ee7

The CI failure in this PR is the same as the CI failure on master:

2022-12-31T14:59:54.9119663Z 101586 lines; 4.555s; 137.285MiB peakmem; proj: /home/runner/work/parsetoml/parsetoml/decoder/decoder.nim; out: /home/runner/work/parsetoml/parsetoml/decoder/decoder [SuccessX]
2022-12-31T14:59:55.1963762Z go: downloading github.com/BurntSushi/toml-test v1.2.1-0.20220607204608-dfd36b634d40
2022-12-31T14:59:55.6193806Z go: downloading zgo.at/zli v0.0.0-20220603144954-fdab8cc4d9d8
2022-12-31T14:59:55.6214572Z go: downloading github.com/BurntSushi/toml v1.1.1-0.20220607004320-6e960699464c
2022-12-31T14:59:55.7104145Z go: downloading github.com/BurntSushi/toml v1.2.1
2022-12-31T14:59:55.7616865Z go: downloading zgo.at/zli v0.0.0-20221012220610-d6a5a841b943
2022-12-31T14:59:55.8452148Z go: downloading golang.org/x/term v0.0.0-20220411215600-e5f449aeb171
2022-12-31T14:59:55.8744567Z go: downloading golang.org/x/term v0.3.0
2022-12-31T14:59:55.8835933Z go: downloading golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1
2022-12-31T14:59:56.1118415Z go: downloading golang.org/x/sys v0.3.0
2022-12-31T14:59:56.5102992Z github.com/BurntSushi/toml/internal
2022-12-31T14:59:56.5215574Z golang.org/x/sys/unix
2022-12-31T14:59:56.5284262Z github.com/BurntSushi/toml
2022-12-31T14:59:56.6381305Z # golang.org/x/sys/unix
2022-12-31T14:59:56.6383324Z ##[error]../../../go/pkg/mod/golang.org/x/[email protected]/unix/syscall.go:83:16: undefined: unsafe.Slice
2022-12-31T14:59:56.6390200Z ##[error]../../../go/pkg/mod/golang.org/x/[email protected]/unix/syscall_linux.go:2256:9: undefined: unsafe.Slice
2022-12-31T14:59:56.6391735Z ##[error]../../../go/pkg/mod/golang.org/x/[email protected]/unix/syscall_unix.go:118:7: undefined: unsafe.Slice
2022-12-31T14:59:56.6496094Z ##[error]../../../go/pkg/mod/golang.org/x/[email protected]/unix/sysvshm_unix.go:33:7: undefined: unsafe.Slice
2022-12-31T14:59:56.6496930Z note: module requires Go 1.17
2022-12-31T14:59:56.9361232Z github.com/BurntSushi/toml-test
2022-12-31T14:59:57.0975317Z stack trace: (most recent call last)
2022-12-31T14:59:57.0976365Z /tmp/nimblecache-946033505/nimscriptapi_2165949961.nim(209, 16)
2022-12-31T14:59:57.0977010Z /home/runner/work/parsetoml/parsetoml/parsetoml.nimble(35, 7) run_toml_test_with_skipsTask
2022-12-31T14:59:57.0977725Z /home/runner/work/parsetoml/parsetoml/nim/lib/system/nimscript.nim(264, 7) exec
2022-12-31T14:59:57.0978875Z /home/runner/work/parsetoml/parsetoml/nim/lib/system/nimscript.nim(264, 7) Error: unhandled exception: FAILED: go get -u -v github.com/BurntSushi/toml-test/cmd/toml-test@master [OSError]
2022-12-31T14:59:57.0999995Z        Tip: 1 messages have been suppressed, use --verbose to show them.
2022-12-31T14:59:57.1000783Z nimscriptwrapper.nim(160) execScript
2022-12-31T14:59:57.1001000Z 
2022-12-31T14:59:57.1001287Z     Error:  Exception raised during nimble script execution
2022-12-31T14:59:57.1004291Z ##[error]Process completed with exit code 1.

Can we fix that?

ee7 avatar Jan 06 '23 12:01 ee7

@PMunch says:

Pretty neat when you find a fairly large project like this and don't recognize any of the contributors

:)

That project is the reason for this PR, by the way.

ee7 avatar Mar 07 '23 16:03 ee7

Yeah I had a poke through your nimble file to see if you used any of my libraries :) Guess I should prioritise this PR, honestly it just completely dropped off my radar at some point..

PMunch avatar Mar 07 '23 22:03 PMunch