(dev) problems about the new `sendList` about hfs HOT 14 CLOSED

I just see there's Readable.unshift. It can "put back" but not that beautiful.

I'll continue anyway. Looking forward to your thoughts :)

from hfs.

would you please open a separate issue for highWatermark?

from hfs.

would you please open a separate issue for highWatermark?

Let me just remove it, Node is confusing me again.
I'd make it clear later. I need more test because I get false stuffs all the way.

One matter is certain: be aware of potential memory problem.

Very sorry about that 😢 And go***mn the docs and internet stuffs that are messy and confused me

from hfs.

I think my migration is done: the commit

... and my use of Readable.unshift here

This can be closed, if you think unshift is good enough 🙂

from hfs.

i hope to find a better solution but had no time to look into this atm

from hfs.

ok, this may not be perfect but maybe it's good enough: i could add a failed property to the Readable.

    function fail(code: any) {
        if (!sse)
            return new ApiError(code)
        list.error(code)
        return Object.assign(list.return, { failed: code }) // allow local code (plugins) to easily read failure code
    }

Another option is to call Readable.destroy(error)

    function fail(code: any) {
        if (!sse)
            return new ApiError(code)
        list.error(code)
        list.return.destroy(code)
        return list.return
    }

This seems cleaner, but I guess you cannot simply "read" the error in this case. You'd probably have to setup a listener for the error event.
Do you want to evaluate these solutions yourself?
We may even do both 🤪

from hfs.

i've just found this
https://nodejs.org/api/stream.html#streamiserroredstream
worth a try?

from hfs.

I tested and I can get error code from "error" event, via that destroy
But then default frontend can't get it (the old problem)

Maybe you can make those sendList operations into a subclass inheriting Readable, and add failed attribute etc:

    class SendListReadable<T> extends Readable {
        failed: number | undefined;
        add(rec: T) { this.push({ add: rec }); }
        // ...
        error(msg: number) {
            this.push({ error: msg });
            this.failed = msg;
        }
    }

And use its instances :)

from hfs.

yeah, why not
but i protected the property, and gave it a more generic name, because i don't know if it's always a fatal failure like for file_list.
Have a look at last commit, if it suits you

from hfs.

Remove this, as it's even destroying the default frontend 🤣
(to reproduce: browse a protected folder without permission, you get 404 by node exception, not 401 file_list without response)

After that everything works... 😁

from hfs.

i can't succeed reproducing the problem.
My frontend doesn't list protected folders unless you have access, so I cannot actually click the folder.
If I, instead, enter the url directly, I get the 401 from the first request, correctly. In this case file_list is actually useless and shouldn't be called, but it is called anyway (should be fixed), and I can see that it transmitted {error:401} as well.
Are you sure about the problem? Any idea?
That line is actually not very important, and leave the connection open even if we are not going to send anything else.
It could easily be removed if we are not sure it is causing no harm.

from hfs.

I tested with Node.js version 16.15.1 (LTS) and 18.3.0 (my local version),
with both linux and wine/exe, (though in this case system doesn't matter)
wIth default frontend,
view protected folder without permission (yes by entering address) --

16: shows login prompt. Page request is 401 and file_list request is 200, inside is error object. Proper.

18: shows listing page but listing process hangs there, with "Stop List" blinking. Page request is 401 but file_list is aborted (no response), and exception of Node in terminal (even when non-DEV), indicating 401 was not handled

Well, another dirty job of Node. They even didn't say what's up in doc update history.

While, in my plugin I have:

await once(readable_list, 'readable');

When "infected" by this, destroy will make it equivalent to:

throw new Error(404); // whatever, abort here

Disaster 😵‍💫

Easy & Direct approach is simply don't destroy...
Edit: maybe push null instead, to actually end it

from hfs.

thank you very much for your useful testing.
You are right, it's a shame there's this different behaviour if it's not documented.
I want to play a bit with it in node18 to see if I understand something more.
null is definitely an option.

from hfs.

        list.error(code)
        setTimeout(() => list.destroy(code))

this is also a solution.
But I think i should go for list.end() instead, that's, you know, the null.
pushed!

from hfs.