typesense-js icon indicating copy to clipboard operation
typesense-js copied to clipboard

Request Cancellation

Open skipjack opened this issue 3 years ago • 5 comments

Description

I'd like to be able to abort requests using the AbortController API...

https://developer.mozilla.org/en-US/docs/Web/API/AbortController

Steps to reproduce

This is a feature request not a bug.

Expected Behavior

There'd be some way to pass an AbortController to each underlying axios call in order to cancel an ongoing request before another is made. This not only saves resources but also helps to avoid race conditions where an older request comes back after a newer one (e.g. when typing fast).

https://axios-http.com/docs/cancellation

Actual Behavior

As far as I can tell, there's no way to do this as of right now using things like .collections(...).documents().search(...). I think we'd need to add an optional second argument to anything using ApiCall

Metadata

Typsense Version: SDK v0.12.1 / Server v0.22.2

OS: MacOS

skipjack avatar Feb 25 '22 03:02 skipjack

Disregard, seems possible already using the second SearchOptions argument...

https://github.com/typesense/typesense-js/blob/master/src/Typesense/Documents.ts#L129-L131

skipjack avatar Feb 25 '22 03:02 skipjack

Works with the latest version (v1.2.1) but detecting the AbortError through axios is a little clunky. May be worth switch to the native fetch API at some point where you can just check error.name === 'AbortError' rather than error.message.includes('aborted'), which feels a bit more fragile.

skipjack avatar Feb 25 '22 04:02 skipjack

I'm also seeing a number of uninformative warnings with undefined that may be related to the cancellation...

image

That may indicate it's not fully supported by the SDK yet? Maybe I should submit a small PR to ignore abort errors on the SDK's side?

skipjack avatar Feb 25 '22 04:02 skipjack

This fails when passing the abortSignal react-query useQuery hook provides, the promise throws.

Request #1645905684416: Request to Node 0 failed due to "undefined undefined"
at node_modules/typesense/lib/Typesense.js:34:728 in _interopRequireWildcard
at node_modules/typesense/lib/Typesense/Configuration.js:80:15 in _isNodeMissingAnyParameters
at node_modules/typesense/lib/Typesense/Configuration.js:106:7 in _setDefaultPortInNode

Request #1645905684416: Sleeping for 0.1s and then retrying request...
at http://10.0.4.12:19000/node_modules/expo/AppEntry.bundle?platform=android&dev=true&hot=false&minify=false:247911:34 in _callee$
at node_modules/typesense/lib/Typesense/Configuration.js:80:15 in _isNodeMissingAnyParameters
at node_modules/typesense/lib/Typesense/Configuration.js:106:7 in _setDefaultPortInNode

update 1:

This is an example on how the signal is being passed:

import { useQuery } from 'react-query';

useQuery(
    ['search', query],
    async ({ signal }) => {
      if (!query.trim() || query.length < 3) {
        return { query, hits: [] };
      }

      const omni = typesense.collections("omni").documents();
      const results = await omni.search({
        q: query,
        query_by: "name, description, labels",
      }, {
        // TODO: Fix query cancelation signal in typesense
        abortSignal: signal
      });

      return {
        query,
        hits: results.hits,
      };
    },
    config
  );

edgarsilva avatar Feb 26 '22 20:02 edgarsilva

Doesn't seem possible for export and exportStream where in my opinion it makes the most sense. Looking at ApiCall get it's totally possible, only place it's missing is in the export, exportStream parameters.

flevi29 avatar Jul 12 '22 06:07 flevi29