razshare / sveltekit-sse

Server Sent Events with SvelteKit
https://www.npmjs.com/package/sveltekit-sse
MIT License
274 stars 9 forks source link

'Controller is already closed' when redirecting to different page #28

Closed CyberNinja2007 closed 7 months ago

CyberNinja2007 commented 7 months ago

Hey there. I'm a newbie in SvelteKit and I've tried to create a simple app with 2 pages - one is for creating messages and the other one is for viewing them. I'm saving data to PosgtreSQL and managing it with pg lib inside a +page.server.ts. In my +server.ts for SSE I need to create an infinite connection with my +page.svelte and when I get notify about new message from pg I need to send it to +page.svelte, where I manage it. I managed to create an infinite connection with timeout:0, but when I go to the creating page from the history page, I get the error: Invalid state: Controller is already closed. How can I fix that? I also tried the example from the README.md with while(true)... but hot the same error when tried to leave the page with connection to the SSE. My code looks like this:

// src/routes/api/messages/+server.ts
import { events } from 'sveltekit-sse';
import { pool } from '$lib/server/db.js';

const updatedRows: string[] = [];

const clientUpdate = await pool.connect();
await clientUpdate.query('LISTEN transactions_update');

export function POST({ locals, request }) {
    return events({
        request,
        headers: {
            'Access-Control-Allow-Origin': '*'
        },
        timeout: 0,
        start({ emit, lock }) {
            if (!locals.authedUser) {
                lock.set(false);
                return function cancel() {
                    console.log('User not authorized.');
                };
            }

                        clientUpdate.on('notification', (notification) => {
                            console.log('updated', notification);
                            if (notification.payload){
                                    const { error } = emit('updated', updateRow);

                    if (error) {
                        console.error('There is an error with sending the updated data', error);
                        return;
                    }
                                }
                        });
        }
    });
} 

// +src/routes/history/+page.svelte

<script lang="ts">
    import type { PageData } from './$types';
    import { source } from 'sveltekit-sse';
    import { onDestroy } from 'svelte';

    const connection = source('/api/messages', {
        close: (event) => {
            console.log('Connection closed', event);
        },
        error: (event) => console.error('Connection error', event)
    });

    const updatedMessagesString = connection.select('updated');

    export let data: PageData;

    $: {
        if ($updatedMessagesString ) {
            console.log('updatedMessagesString ', $updatedMessagesString );
                        data.messages.push(JSON.parse($updatedMessagesString));
        }
    }
</script>
razshare commented 7 months ago

Hi @CyberNinja2007 ,

I just published a fix, try update to v0.8.3.


As a side note, you don't need the timeout:0 to create an infinite connection. In fact, you may want to leave that timeout as is, or set it to a value greater than 0.

What that does is it cleans up zombie events.

Setting it to 0 or a negative value disables this automatic cleanup, which means you have to lock.set(false) yourself manually, which can be difficult to do, because it's difficult to detect when a client aborts an http connection in sveltekit.

More details here.


Let me know if this fix solves your problem.

CyberNinja2007 commented 7 months ago

Yeah, this solved my problem, thank you very much for the fast answer! Hmm, but how can I keep the connection opened? If I remove the timeout 0, the connection close. Should I make a while(true) or a promise?

razshare commented 7 months ago

It should remain opened as long as you don't close your browser tab. Have you tried debugging your server endpoint?

Looking at your code above, maybe this part triggers?

if (!locals.authedUser) {
  lock.set(false);
  return function cancel() {
    console.log('User not authorized.');
  };
}

Or maybe can you see any errors in your console/network inspector?


Here's how I would do it https://github.com/tncrazvan/sveltekit-sse-issue-28-example

git clone https://github.com/tncrazvan/sveltekit-sse-issue-28-example

I replaced your notification event with an interval for simplicity on my side, it should be equivalent for the problem at hand.

CyberNinja2007 commented 7 months ago

Thank you for your example repo. I've modified it to work with my code and managed to find out that if I don't emit something periodically - connection closes or the message just don't showing up. I setted up the timeout on the server on 10 mins and beacon on the client on 1 sec and after 5 mins I created a new message, it didn't show up even on the server side. After it I added your timer on 5 secs, it worked just fine but with one con. I subscribed on the client to 2 messages - date and updated and every 5 secs I got just date, but after I got the message from channel updated, I start to getting date with updated messages every 5 sec from the server with the previous created message until I create a new one and it repeats with that message.

upd: I think that the problem with not getting messages after 5 mins even on server is with my db code, so it's leaved, but the think that every 5 sec I get two messages is still here.

razshare commented 7 months ago

it didn't show up even on the server side

Do you mean it didn't show up in your database event listener? If that's the case, then yes, that seems like a db issue not a library issue.

I setted up the timeout on the server on 10 mins and beacon on the client on 1 sec

That sounds extreme. Those 2 numbers should be fairly close to each other.

The beacon is nothing more than a periodical signal sent to the server to notify that the client is still online.

If the server timeout is set to 10 minutes, then maybe the client beacon should be something like 9 minutes and 50 seconds. They should be really close, with just a small padding to take into account estimated network latency.


I'll make another repo tonight to test out longer running events to make sure everything works fine with numbers as high as half an hour or even several hours, and I'll post it here.

CyberNinja2007 commented 7 months ago

Do you mean it didn't show up in your database event listener? If that's the case, then yes, that seems like a db issue not a library issue.

Yes, it's the another library problem.

The problem I can't deal with is if I emit for example date every second, after I emit neccessary information, I will get it emitted every second with the date message. I have this code:

const interval = setInterval(async function run() {
                emit('date', new Date().toISOString());
                if (updatedRow.length > 0) {
                    updatedRow.forEach((row) => {
                        const { error } = emit('updated', row);
                        if (error) {
                            console.error('Error', error);
                            return;
                        }
                        updatedRow.shift();
                    });
                }
                if (deletedRow.length > 0) {
                    deletedRow.forEach((row) => {
                        const { error } = emit('deleted', row);
                        if (error) {
                            console.error('Error', error);
                            return;
                        }
                        deletedRow.shift();
                    });
                }
            }, 5000);

Here I emit the date every 5 secs and if there are some updated or deleted rows, I also emit them. After I emit some updated or deleted row, I start getting 2 messages every 5 seconds - date with the date and updated/deleted with the last deleted/updated message.

razshare commented 7 months ago

Yes, it's the another library problem.

Ok, in that case I'm closing this issue.