SolidLabResearch / generic-data-viewer

Generic query-based data viewer
MIT License
0 stars 0 forks source link

Comunica engine doesnt resolve promise #20

Closed AronBuzogany closed 1 year ago

AronBuzogany commented 1 year ago

Using test query with id 123 in the config file, currently on branch login-authentication we can reconstruct the following example cases.

The promise seems never to resolve or reject. I have found this by placing console.logs() after both cases, neither of them actually happened.

I have also tested these cases on Comunica jQuery widget. Although we cannot be sure, they seem to result in similar results.

AronBuzogany commented 1 year ago

I have tried creating a new Engine for every query separately, but the same issue occurs

pheyvaer commented 1 year ago

Can you test this with data sources that are actual Solid pods?

AronBuzogany commented 1 year ago

Haven't done this with Solid pods, but I created a custom fetch function that simply throws an error:

function myFetch(arg){
  throw new Error("fail")
}

When using this fetch as the fetch function of myEngine.query the same issue occurs. So when using Solid pods if the fetch function throws an error or the Comunica engine throws an error on the fetch the issue above will occur.

pheyvaer commented 1 year ago

But why would the fetch fail in the first place?

AronBuzogany commented 1 year ago

I haven't yet found a failing example with a Solid pod though and when looking into if pods can disallow authenticated fetch (failing example of this issue, the authenticated fetch fails) I do not find anything on this.

pheyvaer commented 1 year ago

A pod will only say if you have access (200) or not (401 or 403), but the fetch itself should not a throw an error because of that.

AronBuzogany commented 1 year ago

In that case I don't think this issue applies to Solid pods.

pheyvaer commented 1 year ago

Did you already report this in the Comunica repo?

AronBuzogany commented 1 year ago

Did you already report this in the Comunica repo?

I have placed an issue in the Comunica repo: