silverbulletmd / silverbullet

The knowledge tinkerer's notebook
https://silverbullet.md
MIT License
2.36k stars 169 forks source link

Different fetch behavior between client and server #750

Open justyns opened 7 months ago

justyns commented 7 months ago

I have a function in a plug that calls fetch like this, where body is a string (json-encoded object):

    const response = await fetch(
      aiSettings.openAIBaseUrl + "/chat/completions",
      {
        method: "POST",
        headers: headers,
        body: body
      },
    );

On the client in a browser, this works as expected. When run on the server though, the body parameter seems to get dropped somewhere. I was using mitmproxy to confirm body shows up in the request from the client, but not the request from the server.

I see there is some stuff that monkey patches fetch and does some base64 encoding/decoding of the body, but it looks like that should be transparent to the user? Is this a bug, or am I missing something?

zefhemel commented 7 months ago

All this monkey patching business is something I'm contemplating to remove, there's too many subtle issues. In some places I've now switch to use nativeFetch, which is just fetch but subverting the monkey patching stuff. Could you try switching to that and see if that works? To do so, add this import (all this gives you is a type signature: https://github.com/silverbulletmd/silverbullet/blob/main/plug-api/lib/native_fetch.ts):

import "$sb/lib/native_fetch.ts";

And then use nativeFetch instead of fetch everywhere. Here's how I use it: https://github.com/silverbulletmd/silverbullet/blob/main/plugs/federation/federation.ts

zefhemel commented 7 months ago

Ok removing the monkey patch stuff may not be a great option, but try the nativeFetch alternative instead, this should work the same everywhere and I think all LLM providers will set CORS headers (at least OpenAI claim they do).

justyns commented 7 months ago

I can confirm using nativeFetch works fine so far in both cases I was testing, thanks!

Also I didn't really think about it before, but I'm using the SSE.js library for the streaming responses. It looks like this bypasses fetch altogether and uses xhr/XMLHttpRequest() directly.

justyns commented 7 months ago

Follow-up question to this, is there a way to force a request to happen on the server instead of the client? e.g. I tried implementing https://github.com/justyns/silverbullet-ai/pull/23 and it fails because anthropic has no support for cors in their api. We'd have to proxy all requests through the server for it to work.

zefhemel commented 7 months ago

So this is something the monkey-patched fetch is supposed to be for, but yeah…

What you can do is this, which has almost the same effect:

  1. Create a function (as in: a plug function) and set env: server for it, perform the fetch() from here.
  2. Invoke that function (from the client, or anywhere) via system.invokeFunction. This will proxy the request to the server.

This won’t work in sync mode because there everything runs in the client, but this is the best we can do.