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

Handle error thrown by fact #311

Open
robross0606 opened this issue Aug 11, 2022 · 4 comments
Open

Handle error thrown by fact #311

robross0606 opened this issue Aug 11, 2022 · 4 comments

Comments

@robross0606
Copy link

robross0606 commented Aug 11, 2022

I'm trying to figure out what the Engine does it a Fact function throws an exception. As far as I can tell, there is no event handler for this and it doesn't return the error as the Fact value or anything like that. It also doesn't seem to be caught by either of the existing success or failure events. Is this supported in some way or does the entire engine.run() bomb out?

@robross0606
Copy link
Author

robross0606 commented Aug 17, 2022

I have just found that a fact throwing an error will kick back from engine.run():

try {
  await engine.run()
} catch (error) {
  engine.stop()
}

However, the remaining rule facts somehow continue to execute on the node.js process loop outside the await used in the try/catch block. Calling engine.stop() does not stop that execution thought it does set the status of that RULE to 'FINISHED'. Nevertheless, I've found via Jest spy that fact execution seems to continue even after.

Is there any way to prevent this short of never throwing an exception from a fact?

@robross0606
Copy link
Author

robross0606 commented Aug 18, 2022

Turns out the engine keeps a factResultCache on the almanac that has all the promises on it. There are indeed some that are "pending" even after the engine is closed in an error situation. I was able to work around the problem with a special error type which includes the almanac on the thrown error:

      if (error.almanac) {
        // Resolve remaining fact cache to prevent continuing async execution
        await Promise.allSettled(error.almanac.factResultsCache.values())
      }

@robross0606
Copy link
Author

This is effectively a process/memory leak. Can this be included in Milestone 7?

@robross0606
Copy link
Author

robross0606 commented Sep 16, 2023

Another potentially useful feature here would be to have an event handler for "error":

engine.on('error', (event, almanac, error) => {
  // TODO:  Handle errors thrown by facts
})

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

1 participant