Query-Solid icon indicating copy to clipboard operation
Query-Solid copied to clipboard

Errors generated by ldflex are not available to parent application

Open james-martin-jd opened this issue 5 years ago • 11 comments

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.

james-martin-jd avatar Aug 13 '19 21:08 james-martin-jd

May be related to #23.

rubensworks avatar Aug 14 '19 06:08 rubensworks

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.

RubenVerborgh avatar Aug 14 '19 09:08 RubenVerborgh

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)

rubensworks avatar Aug 14 '19 09:08 rubensworks

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?

RubenVerborgh avatar Aug 18 '19 21:08 RubenVerborgh

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)

rubensworks avatar Aug 19 '19 06:08 rubensworks

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

michielbdejong avatar Nov 08 '19 10:11 michielbdejong

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

RubenVerborgh avatar Nov 08 '19 14:11 RubenVerborgh

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

rubensworks avatar Aug 20 '20 09:08 rubensworks

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

rubensworks avatar Aug 20 '20 10:08 rubensworks

(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 avatar Aug 20 '20 11:08 klaasg

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

rubensworks avatar Aug 20 '20 11:08 rubensworks