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

Catch/report on exceptions in match-triple #803

Closed
wants to merge 1 commit into from

Conversation

bplatz
Copy link
Contributor

@bplatz bplatz commented Jun 7, 2024

Exceptions can occur in match-triple that are uncaught and prevent responses (handing up fluree/server, for example). This captures/reports on those.

@bplatz bplatz requested a review from a team June 7, 2024 16:11
(where/match-flake solution tuple db flake)))))
db-alias (:alias db)
triple (where/assign-matched-values tuple solution)]
(if-let [[s p o] (where/compute-sids db triple)]
Copy link
Contributor

@zonotope zonotope Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory match-triple itself shouldn't ever throw any exceptions because it should only stitch together other functions that themselves handle any errors they have with their supplied error channel directly. That's why there was no error handling here originally.

There was an issue that I recently ran across on this line caused by a recent change where the compute-sids function could throw an exception if the namespace for one of the iris in the triple components wasn't registered in the db when the whole function returned nil in that case previously.

I restored the previous behavior in an open pr because it created an inconsistency with how the database handled unknown iris. An unknown iri with a namespace that has already been registered will return an empty result set from a query, but an unknown iri with a namespace that hasn't been registered will now throw an error, but there's no difference between these iris from a user perspective.

This was an issue for the Mondeca dev team because they were executing queries to find out if a subject was known to the ledger or not, and queries that were previously working started timing out with errors in the logs.

I think if we have other places in the code where we'd like to encode an iri/decode an sid but return an exception if the namespaces aren't registered, then we could make a new encode-iri-strict/decode-sid-strict pair for those cases, but i think we should restore the previous behavior of encode-iri/decode-sid and use them here so that we still return nil/[] when a user queries for an iri with an unregistered namespace. If we do that, then I don't think these changes will be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was definitely the exception I was hitting. I think there was another exception as well, but I had this stashed for a bit and can't recall. I'll close it for now, and hopefully you get your other fix merged ASAP.

Uncaught exceptions are causing complete failures in fluree/server - I think we should make sure they all get stomped out. Fluree/server should also be more resilient, I'm not sure why it locks up the entire web app as you'd think other threads could spin up even if one is busy. Separate issue, but related.

@bplatz bplatz closed this Jun 7, 2024
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

Successfully merging this pull request may close these issues.

None yet

2 participants