search

LinkedDataFragments / Client.js

[DEPRECATED] A JavaScript client for Triple Pattern Fragments interfaces.
http://linkeddatafragments.org/
Other
92 stars 35 forks source link

Prevent new requests after abortAll has been called. #40

Closed joachimvh closed 7 years ago

joachimvh commented 7 years ago

If the client received a partial result when abortAll was called it was possible for the process to continue since the relevant metadata might have been received already.

mielvds commented 7 years ago

does this fix #38 ?

joachimvh commented 7 years ago

No, this is to make sure there is a function to just stop the client when required as requested in #39 . It might also fix the issue in #38 if called once all results are returned, but it should not be required to do so.

rubensworks commented 7 years ago

I'm wondering if this should become a warning instead of an error, or if it should completely be silent.

With this fix, the client could crash after abortAll has been called if metadata has already been received. If an external application calls abortAll, it just wants the client to stop producing results, not necessarily stop the whole Node application, which is what would happen if the client crashes because of this error.

@joachimvh What do you think?

joachimvh commented 7 years ago

From what @RubenVerborgh told me, abortAll was sort of intended as a hack anyway to stop the system. It's not really a good way to stop due to the abrutpness, but it at least provides the option.

The problem is that this function can already throw an error even before this fix is applied. Depending on when exactly the function is called a 'socket hang up' error will be thrown because of the socket closing. What this change does is make sure that in the few cases where this error is not thrown (due to timing), another error gets thrown instead to make sure the process stops.

rubensworks commented 7 years ago

Ok, I see, will merge in that case.

Perhaps #39 needs some kind of a clean kill method then. Not sure if it makes sense to implement this in this client, or keep this as a requirement for the new client.

joachimvh commented 7 years ago

Yes a clean kill/stop method would be nice, but would require more invasive changes.

jacoscaz commented 7 years ago

What is the new client? Is it a version of ldf-client or something different?

jacoscaz commented 7 years ago

And yes, I would appreciate a dedicated kill() method very much.

rubensworks commented 7 years ago

What is the new client? Is it a version of ldf-client or something different?

It's something slightly different that will replace the ldf-client in the future. But it's still very much WIP, so not something for the near future.