Recent update to 2.3.1 do not list exception exhaustively

[Clonkk]

After updating, I now have an error message :

/home/rcaillaud/.nimble/pkgs/norm-2.3.1/norm/sqlite.nim(148, 103) template/generic instantiation from here
/home/rcaillaud/.nimble/pkgs/norm-2.3.1/norm/sqlite.nim(173, 6) Error: fromRow(obj, get row) can raise an unlisted exception: IOError
       Tip: 120 messages have been suppressed, use --verbose to show them.

This prevents me from compiling using latest devel compiler.

Using 2.3.0, I don't have this issue

[moigagoo]

@Clonkk Seems like the compiler got smarter in devel and can detect more exceptions than the current stable one :-)

[moigagoo]

I can't reproduce that in Docker. testament won't see installed packages with devel compiler:

$ docker-compose run test tests/sqlite/tdb.nim
[+] Running 1/0
 - Container norm_postgres_1  Running                                                                                                                                                  0.0s
FAIL: tests/sqlite/tdb.nim c                                       ( 2.64 sec)
Test "tests/sqlite/tdb.nim" in category "sqlite"
Failure: reNimcCrash
$ /root/.nimble/bin/nim c --hints:on -d:testing --clearNimblePath --nimblePath:build/deps/pkgs   --nimCache:nimcache/tests/sqlite/tdb.nim_4a8a08f09d37b73795649038408b5f33  tests/sqlite/tdb.nim
Hint: used config file '/root/.choosenim/toolchains/nim-#devel/config/nim.cfg' [Conf]
Hint: used config file '/root/.choosenim/toolchains/nim-#devel/config/config.nims' [Conf]
Hint: used config file '/usr/src/app/tests/config.nims' [Conf]
Hint: used config file '/usr/src/app/tests/sqlite/config.nims' [Conf]
...........................................................................................................
/usr/src/app/src/norm/sqlite.nim(3, 11) Error: cannot open file: ndb/sqlite

FAILURE! total: 1 passed: 0 skipped: 0 failed: 1

[moigagoo]

No idea why but the --clearNimblePath flag that was added to testament makes it not see installed deps.

[Clonkk]

Probably we have to wait until the dust settles with the new Nimble option to generate cfg file with the path to dependencies.

[moigagoo]

Yeah, I'll give it a shot a couple of weeks later. So far all I can advise is don't use Norm with Nim devel ¯\_(ツ)_/¯

Also, this issue inspired me to run the tests with -d:normDebug and it turns out logging adds a potential Exception exception to all procs it's used in.

[dom96]

No idea why but the --clearNimblePath flag that was added to testament makes it not see installed deps.

If you haven't already, you should report this in the Nim repo. If testament is aimed at being used by packages then this kind of breakage shouldn't happen.

[dom96]

Also, I cannot reproduce this. I tried with this sample code, compiles fine on devel for me: https://gist.github.com/dom96/de15982f0e871e83f467fa06e3d8e284

[moigagoo]

@Clonkk could you please provide a minimal code sample the reproduces the issue?

[moigagoo]

If you haven't already, you should report this in the Nim repo. If testament is aimed at being used by packages then this kind of breakage shouldn't happen.

https://github.com/nim-lang/Nim/issues/19017

[Clonkk]

@Clonkk could you please provide a minimal code sample the reproduces the issue?

I had the issue simply compiling sqlite tests. Using Nim v1.6.0 seems to work. I can't use nimble test because of the testament issue

[moigagoo]

I can't use nimble test because of the testament issue

If running the tests with nim r works, there is no issue.

However, this issue exposes another problem: if you compile with -d:normDebug, every proc can raise generic Exception because the logging functions can.

I hate to add Exception to the raises list, it's way too generic. And I also don't like the idea of adding a conditional push at the top.

What are my options here? Any help would be appreciated.

[Clonkk]

Maybe wrap the logging in a proc that re-raise Exception to IOError (for instance) :

proc logQry(qry: string) {.raise...} =
  try:
    debug(qry)
  except:
    raise newException(IOError, getCurrentExceptionMsg())

That should make your logging only raise IOError if I'm not mistaken

[moigagoo]

Unfortunately, it's the debug proc that can raise Exception, so this won't do.

P.S. Factoring logging out to a separate proc is still a good idea on its own.

[Clonkk]

But if you catch Exception and raise IOError, then you can only raise IOError - I assume the compiler can detect such structure.

[moigagoo]

But if you catch Exception and raise IOError, then you can only raise IOError - I assume the compiler can detect such structure.

Hmmm, I'll give it a shot. I thought the compiler just checks what procs are called from this proc and deduces that the parent proc can raise all the exceptions its children can.

[Clonkk]

@moigagoo Sorry to 'necro' this issue; when upgrading Norm version I encountered this issue again and could not reproduce with unit test(again).

After looking into it in more details, I realized it was caused by some custom function defined to handle my own type : Such as (I had several, but it can be simplified as this) :

proc dbType[T](T: typedesc[seq[U]]) : string = "BLOB" 
proc dbValue[U](val : seq[U]): DbValue = # custom serialization protocol that can raise 
proc to[U](dbVal : DbValue, T: typedesc[seq[U]]) : seq[U] = # custom deserialization protocol that can raise

And those functions can indeed raise.

Personnaly, this lead me to the conclusion that it would be better to not specify the raised exception as it clashes with custom serialization / de-serialization proc for custom type.

The alternative would be for me to limit myself to the listed exception (which I currently do as a workaround) but I found that to be quite limitating.

What do you think ?