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

Resubscribing sends empty value #48

Closed
nietsuu opened this issue May 27, 2024 · 12 comments
Closed

Resubscribing sends empty value #48

nietsuu opened this issue May 27, 2024 · 12 comments
Labels
seen I've seen and read this issue and I'll try find some time soon to work on it.

Comments

@nietsuu
Copy link

nietsuu commented May 27, 2024

I'm not really sure how things should work, but whenever I resubscribe after some time:

function subscribe() {
    const sse = source(...).select(...).json();
    sse.subscribe((value) => {
        console.log(value);
    });
}

subscribe();  // Prints correct value
subscribe();  // Still prints correct value
subscribe();  // As long as its called consecutively, it prints the correct value

// HOWEVER when it's called after some time
// even if it's just 100 ms
setTimeout(() => {
    subscribe();  // Prints `undefined` SOMETIMES
    
    // I call source() 3 times (different eventName) on my actual code
    // 2 of them is undefined, and 1 is correct
    // And it's also inconsistent.
    // Sometimes, the other one is correct, sometimes it's the other
    // What I'm basically saying is that it's random.
    // Sometimes even all of them 3 are undefined
}, 100);
@razshare razshare added the seen I've seen and read this issue and I'll try find some time soon to work on it. label May 27, 2024
@razshare
Copy link
Owner

Hello @nietsuu ,
The first value of the store is always set to blank ('').

Which is probably why you're getting an undefined - the internal json parsing fails.

You can check the parsing error yourself.

<script>
  import { source } from 'sveltekit-sse';

  function subscribe() {
    const sse = source('/events')
      .select('message')
      .json(function or({ error, previous, raw }) {
        console.error({ error, raw });
        return previous;
      });
    sse.subscribe((value) => {
      console.log(value);
    });
  }

  subscribe(); // Prints correct value
  subscribe(); // Still prints correct value
  subscribe(); // As long as its called consecutively, it prints the correct value

  // HOWEVER when it's called after some time
  // even if it's just 100 ms
  setTimeout(() => {
    subscribe(); // Prints `undefined` SOMETIMES

    // I call source() 3 times (different eventName) on my actual code
    // 2 of them is undefined, and 1 is correct
    // And it's also inconsistent.
    // Sometimes, the other one is correct, sometimes it's the other
    // What I'm basically saying is that it's random.
    // Sometimes even all of them 3 are undefined
  }, 100);
</script>

image

The reason for this behavior is because the $ syntax is synchronous, while the sse stream is asynchronous, which means it takes time before you get your first valid value from the server.

There is currently no way to do this in svelte

<script>
  import { source } from 'sveltekit-sse';
  const value = source('/events').select('message').json()
</script>

{#await value then result}
    {$result}
{/await}

without changing the signature of the readable store created by source::select::json.
As a matter of fact, even if I were to change the signature to Promise<Readable<...>>, you can't even use locally declared variables with the $ syntax, like $result in the example above.

You can create your own promise resolver

<script context="module">
  /**
   * @typedef MyType
   * @property {string} data
   */
</script>

<script>
  import { onMount } from 'svelte';
  import { source } from 'sveltekit-sse';

  /** @type {Promise<import('svelte/store').Readable<MyType>>} */
  const promise = new Promise(function start(resolve) {
    const store = source('/events').select('message').json();
    const unsubscribe = store.subscribe(function watch(storeValue) {
      if (storeValue) {
        unsubscribe();
        resolve(store);
      }
    });
  });

  onMount(function start() {
    /** @type {false|import('svelte/store').Unsubscriber} */
    let unsubscribe = false;
    promise.then(function run(channel) {
      unsubscribe = channel.subscribe(function watch(value) {
        console.log(value);
      });
    });
    return function stop() {
      if (unsubscribe) {
        unsubscribe();
      }
    };
  });
</script>

Then you won't get the undefined.
image

I'm sure there's a better way to do that using a derived store.

That's a headache though, you're probably better of managing your default values using source::select::json.

Other than that, I can't do much going on snippets of code when it comes to specific issues like these that include timing things out.

If you can reproduce the error as you see it, I'll take another look.

I'll leave this open for now.

@nietsuu
Copy link
Author

nietsuu commented May 28, 2024

Hello @razshare! Thanks for the response.

I understand its asynchronous nature.
I also know that it returns undefined at first because the store has no value yet.

My problem is different than that. I know for a fact that the store has already a value, but it still returns undefined randomly.

My initial message was lacking so I understand the misunderstanding. I'm gonna try to provide as much information now here.

console

My code basically looked like this:

subscribe();
subscribe();

setTimeout(() => {
    subscribe();
}, 1000);

Okay here's an interesting part.
When I set the cache to false, I now seem to get consistent values but there still an undefined:
image

@razshare
Copy link
Owner

razshare commented May 28, 2024

Hello @nietsuu ,
Please provide a reproduction repository so that I can investigate.
I'm not able to reproduce your case using 3 sequential subscribes and a 4th one timed out by 100 milliseconds or 1 second.

From what I'm experiencing everything works as intended.
First 3 subscribes are blank.
The rest have a json value.

Peek 2024-05-28 15-36

And using json() I'm getting the same result, except the value of the store is undefined instead of blank, which is intended.

Peek 2024-05-28 15-40

Without a proper reproduction I cannot help you from the looks of it.

@nietsuu
Copy link
Author

nietsuu commented May 30, 2024

Hello @razshare!

Here's the reproduction repository: https://github.com/nietsuu/test-sveltekit-sse

While I was reproducing it, I found out that the problem starts happening if I emit different eventNames.

src/routes/+server.ts:

produceSSE(["event0", "event1"]);  // undefined problem occurs
produceSSE(["event0"]);  // Works fine

@razshare
Copy link
Owner

Hello @nietsuu

First of all, update to version 0.13.1 to get the latest fixes described below.

I ran your reproduction and I think I figured it out.
The problem isn't on your client, it's on your server.

I'll try to explain myself

First let's start from your producer on the server

image

Note that you're emitting once per event name.

Remember, connections are cached by default, meaning if you source your data AFTER that emit went off, then you're not going to get anything from the stream.

Let's omit the setTimeout for now, and go with this code

image

You'll get this output

image

which is expected - first the undefined, then the server data comes and and we get the actual json value.

Now let's introduce the setTimeout

image

You'll get

image

an extra undefined.

Which is also correct, because your stream has stopped emitting data as of 1 second ago.

Your backend producer is still locked, but it's not sending anything anymore at this point, so the readable gives your the initial default value: undefined.

Note

See note at the bottom regarding this.

Your issue can be solved in two ways - one of which was bugged as of 10 minutes, I just fixed it.

1. The oldest trick in the book - add a query string

It doesn't matter what query string, any will do, as long as its unique in your domain.
image
This will forcefuly open a new stream, executing your server side code again.
And the result is
image
3 undefiend and 3 values, which is correct.

2. The proper way - disabling caching

This was bugged, I just fixed it.
image
image
And the result is the same as above
image

I'm sending you a PR on your reproduction with the query string solution.

Let me know if this addresses your issue.

Note

There is something to be said about that initial undefined.
It's unavoidable the first time a store reads the stream, the value has to be undefined.
But I'm not so sure about subsequent stores that read the stream.
I could cache the last value of the store and return that.
I need to think about this because there's nothing in the W3C standard that suggests anything like this, and I could do more damage than good with such a change.

@nietsuu
Copy link
Author

nietsuu commented May 31, 2024

Hello @razshare,

I think I understand it now. So it actually caches just the connection. It doesn't cache the emitted value itself. That's why I get the undefined value. Is that correct?

Also, I thought all sources of the same eventName share the same store. But in actuality, every source has their own store created. That explains getting undefined on subsequent sources.

In my opinion, I think sources with the same eventName (also of the same route) should share the same store (if possible).
Let me explain:

In my actual codebase, I am not actually resubscribing using setTimeout. The setTimeout was only used to simulate page navigation.
So what happens in my website is, when I navigate to a different route, then I go back, that will trigger a "resubscribe". And as we already know, this will result in undefined value which is a problem.

Making the sources share a store would fix this problem.
Store is a Svelte-specific feature. So even if you implement this, it wouldn't violate the W3C standard right?

Ways I know to fix my problem

Your solution: Disable caching

This works. But this also means re-executing the producer which might not be ideal on some cases. Like in my case, my producer is actually connecting to a WebSocket. I wouldn't want to keep reconnecting to the WebSocket every time I navigate to my page.

Subscribe at the root route so that it will only do it once

This works as well. But in my case, I can't do this because it wouldn't make sense to do so in my case.

Implement my own store as a wrapper to preserve the emitted values

For now, this solution is the best in my case.


Let me know if you know a better solution.
Thanks for the help!

@nietsuu
Copy link
Author

nietsuu commented May 31, 2024

Also, I still don't know why this happens (caching enabled):

Case 1

produceSSE(["event0", "event1"]);  // undefined problem occurs

Case 2

produceSSE(["event0"]);  // Not undefined, but emits the last value emitted evidently from the timestamp

Case 3

produceSSE(["event1", "event0"]);  // Same as Case 2

@razshare
Copy link
Owner

In my opinion, I think sources with the same eventName (also of the same route) should share the same store (if possible).

Yeah this makes sense.
There's not much else to say, I think you're right.

It should be easy to implement, but I can't do it right now, I'll do it in the evening and I'll let you know when it's ready.

@razshare
Copy link
Owner

Also, I still don't know why this happens (caching enabled):

I'll take a look into these cases too

@nietsuu
Copy link
Author

nietsuu commented May 31, 2024

It should be easy to implement, but I can't do it right now, I'll do it in the evening and I'll let you know when it's ready

Great! Thank you.
Take your time, nothing urgent.

razshare added a commit that referenced this issue May 31, 2024
This should make it so that when you subscribe to an already open stream, you obtain the last value of the stream as the first produced value in your subscription.

For more details see #48
@razshare
Copy link
Owner

Hello @nietsuu ,
The fix is ready, please update to version 0.13.2 and let me know if everything works as expected on your end so that I can close this issue.

@nietsuu
Copy link
Author

nietsuu commented Jun 1, 2024

@razshare
It works perfectly! Thank you.

@razshare razshare closed this as completed Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
seen I've seen and read this issue and I'll try find some time soon to work on it.
Projects
None yet
Development

No branches or pull requests

2 participants