-
Notifications
You must be signed in to change notification settings - Fork 15
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
Errors generated by ldflex are not available to parent application #30
Comments
May be related to #23. |
I was told by @james-martin-jd he was using the latest |
I actually think the issue was never fully fixed, when reading the latest comment: #23 (comment) (and also #23 (comment)) |
Reproducible in Node with the following snippet: const { default: data } = require('.');
const webId = 'https://ruben.verborgh.orgz/profile/#me';
(async () => {
let podStoragePath;
try {
podStoragePath = await data[webId].storage;
}
catch(error) {
console.error(error);
}
console.log(podStoragePath);
})(); results in:
So seems to be an error in |
Note to self: look into Feel free to assign this to me. (No guarantees when I'll be able to look into it though) |
I was able to reproduce this in latest master, with the snippet mentioned above. Investigating. |
The root cause is Comunica not passing the error to us; tracking at comunica/comunica#565. |
Thanks for letting us know @klaasg, reopening comunica/comunica#565 |
@klaasg, I see you've removed you comment again. Does this mean that the problem is fixed for you? In any case, after looking into it a bit more, it looks like Comunica is in fact correctly emitting errors in the resulting
|
(To anyone reading this: my comment simply stated the issue is still there.) @rubensworks After encountering this issue I looked into it and saw that comunica/comunica#565 is supposed to be fixed. The problem however is still there so I pointed that out in my comment. It was actually kind of unnecessary because this issue is still open, also meaning it is not yet fixed. |
@klaasg It should have been fixed though in Comunica. So if this still occurs, then I suspect there's something else going on higher up. |
When using LDFlex for reading or writing data, sometimes errors are (legitimately) generated, such as a 401 or 404 error when the resource is not available or the user doesn't have permission. The problem is the errors are not bubbled up to the parent application, so we cannot catch the error to handle it properly. Instead, the error is logged to the console and it silently fails.
For example, I would like to catch an error and display a friendly message in the UI to the user so they know something has gone wrong. In the past we've used solid-auth-client to do a fetch on the resource before using LDFlex, to ensure we have access and the resource exists. This isn't good practice though.
With the most recent 2.6.0 version, I have done a simple query, such as:
const podStoragePath = await data[webId].storage;
Sometimes the user's webId isn't accessible, or it's an invalid webId. I wrapped this code in a try/catch block, but the catch is never triggered.
The text was updated successfully, but these errors were encountered: