Version 2 wont't work using minification

[zeromatic]

https://github.com/00SteinsGate00/Node-MPV/blob/95907f5d3c27fbf0c56c57f59bd9f04da27e24a9/lib/mpv/mpv.js#L77

Parsing the execution stack for "mpv." does not work when the library code is minified and in my case would always yield null as a result and eventually an error.

[AxelTerizaki]

I was going to ask if it worked or not with version 2 but you edited the title :package:

I'm not used to minification, do you have a solution in mind for the regexp ? Or can you elaborate perhaps ? What does that bit of code looks like once minified ? What's the actual error ?

[zeromatic]

the idea behind the code snippet i referenced is to determine whether the function was called via load() or append() - this is also conveniently stated in the inline code comments nearby :). This determination is implemented by parsing the execution stack via regex and looking for the string "mpv.". But with minified code the execution stack looks something like "m.load(...", so the string "mpv" is never found which in turn causes an exception.

[AxelTerizaki]

I see. But looking for just "m." might cause issues. Plus we don't have any way to tell if another module could take that "m." nomenclature in the minified file ? As I said, not familiar at all with minification.

I also don't know if there's a way to tell if we're inside minified code or not.

[zeromatic]

of course just looking for "m." is not a viable solution, there is no telling what a minifier would map the function name to... But since the received information is solely used to produce a "prettier" error message I think it is not worth discussing all that much. A simple null check on stackMatch before doing https://github.com/00SteinsGate00/Node-MPV/blob/95907f5d3c27fbf0c56c57f59bd9f04da27e24a9/lib/mpv/mpv.js#L78, would prevent the error or if you want the old funcitonality fully intact you could explicitly pass the caller as parameter to the function instead of trying to reproduce who was the original caller from the stack ex post.

[AxelTerizaki]

I see your point, yep.

I sadly don't maintain this lib, we'd need @00SteinsGate00 to approve this and he hasn't been around for a while.

If you want though you can try to submit a PR on my fork in the meantime at https://github.com/AxelTerizaki/node-mpv on the node-mpv-2 branch. I publish it on npm as node-mpv-km. That fork is the same as node-mpv here except with two fixes that you'll find on the branch's readme.

[zeromatic]

thanks for the info! i will definitely look into it as i also experienced problems with ipc sockets, probably you solved that already.

[AxelTerizaki]

Yes, there's a problem with this module and latest versions of mpv, and I've made a patch in my fork. :)

[j-holub]

Hey there,

I defended my master thesis yesterday and the past two month I've hardly done anything else but working on my thesis. Thanks for all the patience. I still have some photography client work to finish and deliver but in the following weeks I want to completely clean up this repository, work out the issues and merge the pull requests.

I have to admit, I will need a few hours to get into my own code again, but luckily I commented it a lot.

@AxelTerizaki if you have already patched it in your work you could submit a pull request if you want to. I got time on my hands now to actually work on them :D

[AxelTerizaki]

I haven't done anything on it sadly, so you can look into it to get back into your code :p

And congrats for your thesis.

[j-holub]

I spent a little bit of time on working on a possible fix. It's basically changing the RegEx so something better as parsing the caller als input is not an option for me, because it would change the interface of load() to the outside in a way, that users of the library wouldn't know why.

I tried to get a minified output to see if my fix works, however when using node-minify to minify my code, the error stack did not change at all. It was still the same as bevore, all the whitespaces where there and so on. My fix works if mpv is changed to just m or any other word or single letter.

Could you give me an example of a minified stack trace? Because I wasn't able to produce one.

[zeromatic]

Hi Jan,

As far as i remeber my minifier mapped mpv to "m" so it would be "m.load(" in the stacktrace or the like. I don't know if that actually happens but in a more generic scenario the minifier might even remap the function name to something else. So I would generally advise against relying on this kind of introspection if you want to be on the safe side.

BR

On 21.01.20 12:23, Jan Holub wrote:

I spent a little bit of time on working on a possible fix. It's basically changing the RegEx so something better as parsing the caller als input is not an option for me, because it would change the interface of |load()| to the outside in a way, that users of the library wouldn't know why.

I tried to get a minified output to see if my fix works, however when using node-minify to minify my code, the error stack did not change at all. It was still the same as bevore, all the whitespaces where there and so on. My fix works if mpv is changed to just m or any other word or single letter.

Could you give me an example of a minified stack trace? Because I wasn't able to produce one.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/00SteinsGate00/Node-MPV/issues/60?email_source=notifications&email_token=AOCGZPJW5JIGCBYOM5SWOTTQ63LLXA5CNFSM4J4HBIZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJPM6ZQ#issuecomment-576638822, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOCGZPMZXWACGMUA3OEC2ZTQ63LLXANCNFSM4J4HBIZQ.

[j-holub]

If it's just that, my fix works just fine. It should work for pretty much anything that you write instead of mpv.

I don't see another way for now. Using caller as an argument for load() is not an option for me as I wouldn't want to expose this to users of the library, since load()is directly used by the user and the whole caller thing is just something internal.

Unfortunately the way the IPC communcation protocol of mpv is implemented, it is super hard to find the source of any errors if anything fales. I've spent so many hours to work around that lackluster protocol, I wish it was any easier.