search

LDflex / Query-Solid

Simple access to data in Solid pods through LDflex expressions
https://solid.github.io/query-ldflex/
MIT License
66 stars 15 forks source link

Errors generated by ldflex are not available to parent application #30

Open james-martin-jd opened 5 years ago

james-martin-jd commented 5 years ago

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.

rubensworks commented 5 years ago

May be related to #23.

RubenVerborgh commented 5 years ago

I was told by @james-martin-jd he was using the latest ldflex-comunica, so the fix to https://github.com/solid/query-ldflex/issues/23 should be included.

rubensworks commented 5 years ago

I actually think the issue was never fully fixed, when reading the latest comment: https://github.com/solid/query-ldflex/issues/23#issuecomment-511764226 (and also https://github.com/solid/query-ldflex/issues/23#issuecomment-489513428)

RubenVerborgh commented 5 years ago

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:

(node:39715) UnhandledPromiseRejectionWarning: FetchError: request to https://ruben.verborgh.orgz/profile/ failed, reason: getaddrinfo ENOTFOUND ruben.verborgh.orgz ruben.verborgh.orgz:443
    at ClientRequest.<anonymous> (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/node-fetch/index.js:133:11)
    at ClientRequest.emit (events.js:182:13)
    at TLSSocket.socketErrorListener (_http_client.js:392:9)
    at TLSSocket.emit (events.js:182:13)
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
    at emitWarning (internal/process/promises.js:81:15)
    at emitPromiseRejectionWarnings (internal/process/promises.js:120:9)
    at process._tickCallback (internal/process/next_tick.js:69:34)
(node:39715) FetchError: request to https://ruben.verborgh.orgz/profile/ failed, reason: getaddrinfo ENOTFOUND ruben.verborgh.orgz ruben.verborgh.orgz:443
    at ClientRequest.<anonymous> (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/node-fetch/index.js:133:11)
    at ClientRequest.emit (events.js:182:13)
    at TLSSocket.socketErrorListener (_http_client.js:392:9)
    at TLSSocket.emit (events.js:182:13)
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
(node:39715) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
    at emitDeprecationWarning (internal/process/promises.js:95:13)
    at emitWarning (internal/process/promises.js:88:3)
    at emitPromiseRejectionWarnings (internal/process/promises.js:120:9)
    at process._tickCallback (internal/process/next_tick.js:69:34)
(node:39715) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 7)
    at handledRejection (internal/process/promises.js:57:23)
    at handler (internal/process/promises.js:23:14)
    at Promise.then (<anonymous>)
    at MediatedLinkedRdfSourcesAsyncRdfIterator._read (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/@comunica/actor-rdf-resolve-quad-pattern-hypermedia/lib/LinkedRdfSourcesAsyncRdfIterator.js:55:18)
    at MediatedLinkedRdfSourcesAsyncRdfIterator.BufferedIterator._fillBuffer (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/asynciterator/asynciterator.js:840:10)
    at Immediate.fillBufferAsyncCallback (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/asynciterator/asynciterator.js:872:8)
    at runCallback (timers.js:706:11)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)
(node:39715) UnhandledPromiseRejectionWarning: FetchError: request to https://ruben.verborgh.orgz/profile/ failed, reason: getaddrinfo ENOTFOUND ruben.verborgh.orgz ruben.verborgh.orgz:443
    at ClientRequest.<anonymous> (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/node-fetch/index.js:133:11)
    at ClientRequest.emit (events.js:182:13)
    at TLSSocket.socketErrorListener (_http_client.js:392:9)
    at TLSSocket.emit (events.js:182:13)
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)
    at emitWarning (internal/process/promises.js:81:15)
    at emitPromiseRejectionWarnings (internal/process/promises.js:120:9)
    at process._tickCallback (internal/process/next_tick.js:69:34)
(node:39715) FetchError: request to https://ruben.verborgh.orgz/profile/ failed, reason: getaddrinfo ENOTFOUND ruben.verborgh.orgz ruben.verborgh.orgz:443
    at ClientRequest.<anonymous> (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/node-fetch/index.js:133:11)
    at ClientRequest.emit (events.js:182:13)
    at TLSSocket.socketErrorListener (_http_client.js:392:9)
    at TLSSocket.emit (events.js:182:13)
    at emitErrorNT (internal/streams/destroy.js:82:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
    at process._tickCallback (internal/process/next_tick.js:63:19)

So seems to be an error in MediatedLinkedRdfSourcesAsyncRdfIterator._read, @rubensworks?

rubensworks commented 5 years ago

MediatedLinkedRdfSourcesAsyncRdfIterator and LinkedRdfSourcesAsyncRdfIterator seem to properly forward promise rejections to error events in the stream. It is however possible that errors are not being forwarded to transformed streams properly higher up in the chain.

Note to self: look into ActorQueryOperationQuadpattern.

Feel free to assign this to me. (No guarantees when I'll be able to look into it though)

michielbdejong commented 5 years ago

I was able to reproduce this in latest master, with the snippet mentioned above. Investigating.

RubenVerborgh commented 5 years ago

The root cause is Comunica not passing the error to us; tracking at https://github.com/comunica/comunica/issues/565.

rubensworks commented 4 years ago

Thanks for letting us know @klaasg, reopening https://github.com/comunica/comunica/issues/565

rubensworks commented 4 years ago

@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 bindingsStream, which LDflex is listening to.

Node is however still printing UnhandledPromiseRejectionWarning to stderr, which should not be there. (was a bug in my testing environment)

klaasg commented 4 years ago

(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.

rubensworks commented 4 years ago

@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.