tursodatabase / libsql-client-ts

TypeScript/JavaScript client API for libSQL
https://docs.turso.tech/sdk/ts/quickstart
MIT License
226 stars 32 forks source link

Query hangs forever on Cloudflare Worker local dev #29

Closed arjunyel closed 1 year ago

arjunyel commented 1 year ago

Discord thread: https://discord.com/channels/933071162680958986/1100593117075345449/1100593117075345449

Running this code on Cloudflare Workers local dev hangs forever

import { createClient } from "@libsql/client/web";

await createClient({
  url: context.TURSO_DATABASE_URL,
  authToken: context.TURSO_AUTH_TOKEN,
}).execute(`select * from users`);

Debugging shows its stuck on this line but can't debug further because it just hangs, doesn't throw an error https://github.com/libsql/libsql-client-ts/blob/358767835750c69df3073d0f43bb7f9e2d8cc34f/src/hrana.ts#L64

Work around for now is to use import { createClient } from "@libsql/client/http"; instead of import { createClient } from "@libsql/client/web";

Thank you!

CodingDoug commented 1 year ago

Could you try assigning the result of createClient to a variable called client, and calling client.close() before the worker finishes handling the request? The client should be both created and closed within the body of the worker. Without that, the client will try to keep the websocket open, which might not work for very long in a worker environment. I suggest calling close() in the finally block of a try/catch/finally to ensure that it happens in all cases.

If that's the case, then using the http import is probably better for simple queries than using the web import because http won't require so much back-and-forth communication with sqld compared to websockets, further reducing latency. The downside is that this eliminates your ability to do interactive transactions.

arjunyel commented 1 year ago

Hi @CodingDoug, big fan of you all the way from your Firebase days :D

The person in discord also suggested that but I think the issue is deeper than that. The Websocket isn't timing out since this is a request as soon as the server starts up. So I'm thinking it's broken totally on Cloudflare Workers?

arjunyel commented 1 year ago

Since I am awaiting the result of the execute it never gets to a point where I can call close(), it's stuck on the await

CodingDoug commented 1 year ago

I tried this myself using wrangler (and used that to write the documentation) so it should work locally. So maybe this is just an issue when deploying to a managed worker (yes, admitting that I might not have done that!).

arjunyel commented 1 year ago

I tried this myself using wrangler (and used that to write the documentation) so it should work locally. So maybe this is just an issue when deploying to a managed worker (yes, admitting that I might not have done that!).

I am developing locally, haven't tried deploying yet. I don't think it should matter but this is actually happening in a Cloudflare Pages function with Remix. I don't think that should effect it though because it should just be using workers under the hood?

CodingDoug commented 1 year ago

I'm not familiar with Remix. So I think it would help if we could see a complete, minimal example that reproduces the issue, so we can run it and try it ourselves.

arjunyel commented 1 year ago

Here is a reproduction https://github.com/arjunyel/turso-remix-bug

  1. npm install

  2. Rename .dev.vars.example to .dev.vars and fill out with Turso variables

  3. Modify the query in app/routes/_index.tsx

  4. npm run dev

  5. Try to navigate to http://127.0.0.1:8788/, it hangs forever

meso commented 1 year ago

Hi,

Are you using any antivirus software? I also struggled with the same issue, and in my case, it seemed that the antivirus software was causing the problem. I was using ESET Endpoint Antivirus on macOS Ventura 13.3.1. I hope this helps.

arjunyel commented 1 year ago

Hi,

Are you using any antivirus software? I also struggled with the same issue, and in my case, it seemed that the antivirus software was causing the problem. I was using ESET Endpoint Antivirus on macOS Ventura 13.3.1. I hope this helps.

Hi friend, I am not but I am also on Mac. Could you reproduce my issue with these steps? https://github.com/libsql/libsql-client-ts/issues/29#issuecomment-1522737452

honzasp commented 1 year ago

Thank you very much @arjunyel for the reproducing example! I was able to reproduce and debug the issue locally and I found the root cause: it seems that in Wrangler local environment, assigning an event handler to WebSocket.onopen (and other websocket events) does not work, we need to use WebSocket.addEventListener(). I will release a new version of @libsql/hrana-client with the fix.

honzasp commented 1 year ago

I just released version 0.3.8 of @libsql/hrana-client (which is a dependency of @libsql/client). Could you please try updating your dependencies using npm update and checking whether the issue is resolved? The reproducing example now works for me.

atjeff commented 1 year ago

I encountered this a week or two ago as well, thought I was going nuts. Thank you for the new version

honzasp commented 1 year ago

I'm sorry for the troubles that this issue caused for you! I will try to add automatic tests of the client library on Cloudflare Workers and in Wrangler, to make sure that problems like this don't happen again.

arjunyel commented 1 year ago

Can confirm 0.3.8 of @libsql/hrana-client fixes it, thank you so much! As a last wrap up note where can I read more about Doug's earlier message?

Could you try assigning the result of createClient to a variable called client, and calling client.close() before the worker finishes handling the request? The client should be both created and closed within the body of the worker. Without that, the client will try to keep the websocket open, which might not work for very long in a worker environment. I suggest calling close() in the finally block of a try/catch/finally to ensure that it happens in all cases.

If that's the case, then using the http import is probably better for simple queries than using the web import because http won't require so much back-and-forth communication with sqld compared to websockets, further reducing latency. The downside is that this eliminates your ability to do interactive transactions.

CodingDoug commented 1 year ago

@arjunyel I don't think there are any existing reading materials for either topic (the effect of calling close() the client or the fact that http might be better than websockets in some scenarios). Both sound like good topics for discussion on Discord.

ben-xD commented 1 year ago

I'm using @libsql/client==^0.1.6 which uses @libsql/hrana-client==0.3.10 and I've got this issue when using the import import { createClient } from '@libsql/client/web';. Switching to import { createClient } from '@libsql/client/http'; fixed it.

Therefore I think this bug still exists? 🤔

arjunyel commented 1 year ago

Talking to the team I think the Websocket part is getting removed. Use http for now

honzasp commented 1 year ago

Hi Ben, thank you for your report! I released a new version 0.3.11 of @libsql/hrana-client (which is a dependency of this package), which should fix your issue on Cloudflare Workers. Can you please give it a try? We seem to have hit yet another problem with the Cloudflare implementation of WebSockets :/

There are no immediate plans to remove WebSockets from the client, because the HTTP API does not support transactions. But if you don't use transactions, then HTTP should work just fine for you.

ben-xD commented 1 year ago

It works now @honzasp, thanks :) 🙏

honzasp commented 1 year ago

I'm glad that it helped! :)