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

Fetch errors do not result in a promise rejection #23

Closed jaxoncreed closed 5 years ago

jaxoncreed commented 5 years ago

When doing an asynchronous request, try/catch statements should properly catch errors, but they do not.

Example:

When running this code via a web-browswer on localhost, Tim's pod disallows requests coming from the unregistered localhost app. This is all fine, but the resulting error it triggers cannot be caught.

    try {
      const timName = (await data['http://www.w3.org/People/Berners-Lee/card#i'].vcard_fn).value;
      console.log(timName);
    } catch(err) {
      console.log('this should be caught')
      console.error(err);
    }

This yields Uncaught (in promise) TypeError: Failed to fetch

The same result will happen if I try to retrieve an improperly formatted ttl file

RubenVerborgh commented 5 years ago

Note: the above likely should not yield any errors ever, in whatever way.

rubensworks commented 5 years ago

Comunica is probably the root cause of this problem, see https://github.com/comunica/comunica/issues/381

RubenVerborgh commented 5 years ago

Root cause is apparently https://github.com/RubenVerborgh/LDflex-Comunica/pull/13.

@jaxoncreed Could you check how the above works for you with the proposed fix there?

RubenVerborgh commented 5 years ago

After quick testing with the original code:

So that discrepancy is also a problem.

RubenVerborgh commented 5 years ago

After https://github.com/RubenVerborgh/LDflex-Comunica/pull/13, also two different behaviors:

Especially the second thing is a problem. If Chrome does not return anything to us on failure, we'd need to resolve to a timeout.

rubensworks commented 5 years ago

I just tested the example above in Chrome 72.0.3626.121, and err is properly caught every time.

However, when I test with an unknown domain, no error is caught at all, but the promise rejects, and only the very first time:

Screen Shot 2019-05-06 at 08 30 21

@RubenVerborgh Are you aware of any caching going on in LDflex? Comunica only caches RDF sources with 200 status codes (unless there's a bug somewhere).

akashdeep-singh commented 5 years ago

Has there been any update on this? I've been getting it on Firefox and Chrome for 403 errors. This is the line it originates from: https://github.com/comunica/comunica/blob/85722d73f1109dcf6042b69e4893b59c84cf3096/packages/actor-rdf-dereference-http-parse/lib/ActorRdfDereferenceHttpParseBase.ts#L59

RubenVerborgh commented 5 years ago

The same problem is also present in Node, where the following code stops:

const ruben = data['https://ruben.verborgh.org/profile/#me'];
showPerson(ruben)

async function showPerson(person) {
  console.log(`This person is ${await person.label}`);
  console.log(`${await person.givenName} is friends with:`);
  for await (const myFriend of person.friends) {
    console.log(`- ${myFriend}`);
    for await (const otherFriend of data[`${myFriend}`].friends)
      console.log(`  - ${otherFriend}`);
  }
}

The error is

(node:14373) UnhandledPromiseRejectionWarning: Error: Could not retrieve http://www.informatik.uni-leipzig.de/~auer/foaf.rdf (404)
    at ActorRdfDereferenceHttpParse.run (/Users/ruben/Documents/UGent/Solid/solid-query-ldflex/node_modules/@comunica/actor-rdf-dereference-http-parse/lib/ActorRdfDereferenceHttpParse.js:37:19)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:14373) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1099)
(node:14373) [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.
RubenVerborgh commented 5 years ago

In https://github.com/solid/query-ldflex/issues/30, we are unsure whether this issue was fully fixed. Might need reproducible test cases for browsers.

michielbdejong commented 4 years ago

See mention there, this bug still seems to be affecting the Solid SDK?

RubenVerborgh commented 4 years ago

Indeed; tracking this in #30.