Skip to content

Conversation

@danielrhodes
Copy link

@danielrhodes danielrhodes commented Apr 10, 2023

I was recently using this on a Cloudflare Worker and ran into this error: TypeError: Illegal invocation.

In order to investigate the issue, which might be related to the way the library was structured and its use of this, I took the opportunity to upgrade it to Typescript.

It passes tests, but since I'm not a deep user of Replicate yet I haven't been able to verify that other things work.

The API should be the same. However, I wasn't able to find good typings for the training endpoints or find them in the documentation on Replicate's API docs.

@mattt
Copy link
Contributor

mattt commented Apr 10, 2023

Thanks so much for taking the time to look into this, @danielrhodes.

First off, apologies for not having correct type information for trainings. I could've sworn I declared that in #35, but I was mistaken. #45 adds the missing type declaration. That's now available in v0.9.1.

As far as converting the project to TypeScript: We love TypeScript for application code. For a library like this, we believe that writing in JavaScript and providing .d.ts type descriptions for TypeScript consumers offers the best combination of portability and usability. We've found that it's easier to reason about any problems that pop up when the JS code being run matches what's being written exactly, without a transpilation step in the middle.

To that end, I'd like to get to the bottom of that TypeError: Illegal invocation you're seeing. Can you please update to the latest version and let me know if that changed anything?

@mattt mattt closed this Apr 10, 2023
@danielrhodes
Copy link
Author

Unfortunately that error is all I have. It's running inside a Cloudflare Worker, which is basically V8, if that helps. It does not happen in my TS version though. In terms of updating to 0.9.1, that update only affects the the types but that isn't included in what gets bundled and shipped to production.

If I were to make a guess, it's some combination of:

https://github.com/replicate/replicate-javascript/blob/main/index.js#L40 or
https://github.com/replicate/replicate-javascript/blob/main/index.js#L136

If you're assigning fetch to this, and this is actually the parent, there does seem to be some room for the wires to get crossed. It's possible referencing fetch from globalThis would solve it.

Either that, or one of the calls to this.wait or this.request.

@mattt
Copy link
Contributor

mattt commented Apr 11, 2023

@danielrhodes Alright, I think I have a handle on what's going on. I think you're right that the issue has to do with how fetch is being resolved.

Can you please try updating your code to add this line after you initialize the Replicate client?

replicate.fetch = globalThis.fetch

Or update to version 0.9.2 and pass this as an option in your initializer:

  import Replicate from "replicate";
  
  const replicate = new Replicate({
    // get your token from https://replicate.com/account
    auth: process.env.REPLICATE_API_TOKEN,
+   fetch: globalThis.fetch
  });

I also have PR #49, which makes this the default instead of unqualified fetch. If this change works for you, I'll feel more confident about merging this.

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

Successfully merging this pull request may close these issues.

2 participants