neume-network / strategies

Indexing strategies for the neume network.
GNU General Public License v3.0
11 stars 7 forks source link

add try-catch block in `extract` function of lifecycle.mjs #272

Closed il3ven closed 1 year ago

il3ven commented 1 year ago

There can be unhandled errors in strategies. These errors can bubble up and cause premature exit and UnhandledPromiseRejection. To prevent this error and properly reject promise returned by the extract function this commit adds a try-catch block. Also, adds two new test cases for the above described scenario.

The UnhandledPromiseRejection can be seen in neume-network/data#42 where an error is generated inside the update function of call-block-logs-extractor. https://github.com/neume-network/data/runs/8234616425?check_suite_focus=true#step:10:37

For this commit to work properly we also need neume-network/core#85. Without it the extract function will reject and it's error will bubble up to neume.mjs but the application still won't close because the worker is still running.

TimDaub commented 1 year ago

I don't know how to say this but using a try block over more than one statement is illegal in our code base. Can we fix this problem differently? What's the reason we want to pack all of this code in try catch?

TimDaub commented 1 year ago

The UnhandledPromiseRejection can be seen in neume-network/data#42 where an error is generated inside the update function of call-block-logs-extractor. https://github.com/neume-network/data/runs/8234616425?check_suite_focus=true#step:10:37

OK but if an error is in update, then wouldn't it suffice to surround the update function with a try catch?

TimDaub commented 1 year ago
node --help

--unhandled-rejections=...  define unhandled rejections behavior.
                              Options are 'strict' (always raise an
                              error), 'throw' (raise an error unless
                              'unhandledRejection' hook is set),
                              'warn' (log a warning), 'none' (silence
                              warnings), 'warn-with-error-code' (log a
                              warning and set exit code 1 unless
                              'unhandledRejection' hook is set).
il3ven commented 1 year ago

If we use --unhandled-rejections=strict then we won't need this try-catch or we can use Node v16 which has this turned on by default.

TimDaub commented 1 year ago

let's use --unhandled-rejections=strict for now and then subsequently upgrade to node v16

il3ven commented 1 year ago

let's use --unhandled-rejections=strict for now and then subsequently upgrade to node v16

I will close this PR and open a PR in neume-network/core and neume-network/data to add --unhandled-rejections=strict.

TimDaub commented 1 year ago

kk thx