Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request Cancellation #112

Open
skipjack opened this issue Feb 25, 2022 · 5 comments
Open

Request Cancellation #112

skipjack opened this issue Feb 25, 2022 · 5 comments

Comments

@skipjack
Copy link

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
Copy link
Author

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

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

@skipjack
Copy link
Author

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
Copy link
Author

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 skipjack reopened this Feb 25, 2022
@edgarsilva
Copy link

edgarsilva commented Feb 26, 2022

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

@flevi29
Copy link

flevi29 commented Jul 12, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants