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

export axios instance #254

Open
zhihengGet opened this issue Jan 7, 2025 · 4 comments
Open

export axios instance #254

zhihengGet opened this issue Jan 7, 2025 · 4 comments

Comments

@zhihengGet
Copy link

Description

please exportthe axios so we can change the axio config i.e interceptor or change to use fetch

Steps to reproduce

Expected Behavior

Actual Behavior

Metadata

Typesense Version:

OS:

@tharropoulos
Copy link
Contributor

Hey there, support for overriding the axios adapter is available in v.2.0.0-14

@isaacrowntree
Copy link

isaacrowntree commented Jan 17, 2025

@tharropoulos I can confirm this version doesn't work in Cloudflare Pages for me.

I'm getting this error locally and in production:

Request #1737087480828: Request to Node 0 failed due to " Illegal invocation: function called with incorrect `this` reference. See https://developers.cloudflare.com/workers/observability/errors/#illegal-invocation-errors for details."

server config:

const server = {
  axiosAdapter: 'fetch',
  apiKey: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_KEY || '',
  nearestNode: {
    host: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_HOST || '',
    port: 443,
    protocol: 'https',
  },
  nodes: [
    {
      host: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_HOST_NODE1 || '',
      port: 443,
      protocol: 'https',
    },
    {
      host: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_HOST_NODE2 || '',
      port: 443,
      protocol: 'https',
    },
    {
      host: process.env.NEXT_PUBLIC_TYPESENSE_SEARCH_HOST_NODE3 || '',
      port: 443,
      protocol: 'https',
    },
  ],
  connectionTimeoutSeconds: 2,
}

Typesense version: "typesense": "^2.0.0-14",

I would much prefer an approach like the SWR NPM package where I can specify the fetcher function I want to use. This is incredibly important for Cloudflare Workers.

class TypesenseClient {
  apiKey: string
  node: string
  collection = ''

  constructor(config: typeof server) {
    this.apiKey = config.apiKey
    this.node = 'https://' + config.nearestNode.host
  }

  collections(collection: string) {
    this.collection = collection
    return this
  }

  documents() {
    return this
  }

  async search(query: TypesenseQuery) {
    const url = new URL(
      `${this.node}/collections/${this.collection}/documents/search`,
    )
    url.searchParams.append('q', query.q)
    url.searchParams.append('query_by', query.query_by)
    url.searchParams.append('sort_by', query.sort_by)
    url.searchParams.append('per_page', query.per_page)
    url.searchParams.append('filter_by', query.filter_by)

    return await (
      await fetch(url.toString(), {
        headers: {
          'X-TYPESENSE-API-KEY': this.apiKey,
        },
      })
    ).json()
  }
}

An example class I just whipped up quickly to get myself going so I can honor the contract you have in your project and quickly and easily delete my class and replace it with yours when/if you ever allow us to maintain a different fetch implementation.

@tharropoulos
Copy link
Contributor

I'm getting this error locally and in production:

Request #1737087480828: Request to Node 0 failed due to " Illegal invocation: function called with incorrect `this` reference. See https://developers.cloudflare.com/workers/observability/errors/#illegal-invocation-errors for details."

(From the link in the stack trace):
The "Illegal invocation" error in Cloudflare Workers happens when a function that relies on its this context (like fetch) loses its binding. This is a common JavaScript behavior - when methods are passed around as standalone functions, they can lose their original this context.
In our case, when passing 'fetch' as a string to the axios adapter, the fetch function gets detached from its original globalThis context. When it's later called, it doesn't have the correct this binding, causing the "Illegal invocation" error.

To fix this, we've added a new getAdapter method that:

  • Detects if we're in a Cloudflare Workers environment
    • If so, it explicitly binds the adapter to globalThis using .bind(globalThis)
  • For non-Cloudflare environments or directly provided function adapters, it uses them as-is

Could you give that a try and see if that address the issue?

I would much prefer an approach like the SWR NPM package where I can specify the fetcher function I want to use. This is incredibly important for Cloudflare Workers.

In regards to the fetcher, in SWR's case, the fetcher is basically any function that contains the same shape of data as the first arg passed into the hook, and returns a promise of said data. It's a lot different than what happens in this client, as the underlying interfaces are using explicit functions for fetching the data. We would instead need to build adapters for each HTTP library to be used underneath.

@tharropoulos
Copy link
Contributor

@isaacrowntree Did you get the time to check it out?

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