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

Exception handling during storing a value #221

Closed
ppedziwiatr opened this issue Aug 10, 2022 · 4 comments
Closed

Exception handling during storing a value #221

ppedziwiatr opened this issue Aug 10, 2022 · 4 comments

Comments

@ppedziwiatr
Copy link

ppedziwiatr commented Aug 10, 2022

In case of an error is thrown during saving/putting a value, e.g.

try {
  const contractCache = this.db.sublevel<string, any>(stateCacheKey.contractTxId, {valueEncoding: 'json'});
  await contractCache.put(stateCacheKey.sortKey, value);
} catch (e) {
  console.error(e);
}

the error is not being catched by the surrounding try-catch in the above code - but somehow propagated to the "top-level" - and as a result causes the node process to exit (error Command failed with exit code 1.), e.g.:

/Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/level-transcoder/lib/formats.js:81
      encode: (data) => Buffer.from(this.encode(data), 'utf8'),
                                         ^
TypeError: Do not know how to serialize a BigInt
    at UTF8Format.stringify (<anonymous>)
    at BufferFormat.encode (/Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/level-transcoder/lib/formats.js:81:42)
    at AbstractSublevel.put (/Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/abstract-level/abstract-level.js:441:39)
    at /Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/abstract-level/abstract-level.js:415:29
    at AbstractSublevel.[undefer] (/Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/abstract-level/abstract-level.js:755:7)
    at /Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/abstract-level/abstract-level.js:168:23
    at Immediate.maybeOpened (/Users/piotrpedziwiatr/projects/redstone-smartcontracts/node_modules/abstract-level/abstract-level.js:138:9)
    at processImmediate (node:internal/timers:471:21)
error Command failed with exit code 1.

Such error can be only catched by:

process
    .on('uncaughtException', err => {
      logger.error('Uncaught Exception thrown', err);
    });

but it's not an option in this case, as it completely breaks our node application execution.

Why does level behaves this way? is it by design?
The current behaviour makes it unusable in a server-side, node.js applications.

@vweevers
Copy link
Member

The value has an invalid type. So it's not an error that should be catchable IMO, because it's a bug in your application. Same as:

$ node
> JSON.stringify(BigInt(123))
TypeError: Do not know how to serialize a BigInt

This level behavior is not entirely by design, just a result of encoding happening synchronously, but I can't say it's a bad behavior. Once we fully move to a promise-based implementation (rather than using callbacks internally, where only the asynchronous steps are "captured" by the promise, and synchronous steps instead raise uncaught exceptions) every error would be catched like you desire. But in my personal opinion it's one of the major downsides of Promises.

@ppedziwiatr
Copy link
Author

ppedziwiatr commented Aug 11, 2022

Hey, thanks for your response.

I believe it's should be always up to the application developer (no the library developer) to decide, which exceptions should be considered as "fatal" and which exceptions should stop the application immediately.

Our application ("smart contracts execution node") evaluates states of the smart contracts (which are stored and loaded from the Arweave blockchain). Each smart contract is a javascript or wasm code.
If it is a javascript code, it is being run inside https://github.com/laverdet/isolated-vm (because we can't obviously trust external code and we need to have the ability to sandbox the smart contract execution environment).

This works really nice, but after evaluating the state - we're storing it in the node's cache - which is using the level library.
If the state storing operation fails - I want to simply move to the next contract (and maybe mark the given contract as blacklisted) - but I really don't want the whole application to stop (or at least to be thrown away from the "main" application loop).
So if I'm wrapping the "put" operation in a try-catch - that's my design choice - and I expect to catch all errors that may be thrown during put (and then - depending on the type of the error - decide what to do next).
Different behaviour is unexpected in this case - and - at least in our case - really "destructive".

I believe that the issue with handling the exception should be noted in the docs...

Is there any timeline to moving to a fully promise-based implementation?

@vweevers
Copy link
Member

Is there any timeline to moving to a fully promise-based implementation?

None at this time.

There might be something more specific going on here. I haven't tested it, but a workaround could be explicitly opening the db before you use it:

await db.open()
await db.put(..)

This ensures the put operation isn't deferred until the db has opened itself, which means the operation (including encoding of the value) will happen inside your try/catch.

@ppedziwiatr
Copy link
Author

This ensures the put operation isn't deferred until the db has opened itself, which means the operation (including encoding of the value) will happen inside your try/catch.

Thanks! This workaround fixes my issue. Appreciate your time and help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants