Compatibility issue with plugin Local REST API

[yopsolo79]

Since the latest version (0.15.8) the plugin Local REST API stopped working. More details is provided here : https://github.com/coddingtonbear/obsidian-local-rest-api/issues/79#issuecomment-1761175261 https://github.com/coddingtonbear/obsidian-local-rest-api/issues/79#issuecomment-1761550589 The author said he could help https://github.com/esodesod I was using version 0.15.7 without problem as the other users mentioned on the post.

[Sergei-Korneev]

I do not use those plugins, so I have not clue that might have happen. I also cannot see the "steps to reproduce" section etc in your message. I followed your links and there were conversation about two plugins I guess "Obsidian Web" / "Local Rest API" So if you guys sort out how to fix it I will add a patch.

[57r31]

Compatibility issue confirmed Caused probably by etag package - that's what returned from Local REST Api plugin when trying to download a certificate

TypeError: argument entity must be string, Buffer, or fs.Stats
    at etag (plugin:obsidian-local-rest-api:39419:15)
    at generateETag (plugin:obsidian-local-rest-api:40374:16)
    at ServerResponse.send2 [as send] (plugin:obsidian-local-rest-api:43551:20)
    at ServerResponse.json (plugin:obsidian-local-rest-api:43579:19)
    at RequestHandler.returnCannedResponse (plugin:obsidian-local-rest-api:45578:63)
    at RequestHandler.eval (plugin:obsidian-local-rest-api:46188:12)
    at Generator.next (<anonymous>)
    at eval (plugin:obsidian-local-rest-api:79:61)
    at new Promise (<anonymous>)
    at __async (plugin:obsidian-local-rest-api:63:10)```

[57r31]

actually it's not etag problem it's Buffer.isBuffer()

god, I hate JS so much it's really easy to debug tho

[57r31]

so I added a string

Buffer.isBuffer = (e) => e.__proto__ instanceof Uint8Array

into .obsidian\plugins\obsidian-local-images-plus\main.js file and plugins now work together

[Sergei-Korneev]

@57r31 Hi, probably it something related to historical difference between Buffer and Uint8Array in node js, lol. Keep on researching.

[57r31]

@Sergei-Korneev maybe fix here?

[Sergei-Korneev]

@57r31 Mate, just for information, You are showing me compiled code of my plugin and its dependencies. Are we going to patch complied code ?

[57r31]

patch works for me lol, but you may want to get to bottom of it that entity buffer object does not have _isBuffer prop. probably you can just remove buffer dep from package.json, or exclude it from bundle, it's embedded in node already

[Sergei-Korneev]

probably you can just remove buffer

Oh rap... I can not even remember why it is there in dependencies. I basically do not have time to test the plugin on compatibility.

You guys can run npm uninstall buffer and rebuild from source to check if it is the exact place where evil reside.

[esodesod]

actually it's not etag problem it's Buffer.isBuffer()

god, I hate JS so much it's really easy to debug tho

Thanks for the details!

I got into the !Buffer.isBuffer(entity)-track yesterday (noticed that by removing the (entity) in Local REST API plugin, it did not throw the TypeError, hence somehow related to the Buffer and entity), but I did not have the time to find the relevant parts in Local Images Plus 0.15.8, even though I noticed it was "new" in 0.15.8 vs 0.15.7.

Anyway, I'm no developer (which I guess should be a "prerequisite" for troubleshooting the code here 🙈), but I love both plugins, hence wanting to help, as much as I can.

Please let me know if I can assist (somehow).

[Sergei-Korneev]

@esodesod Ok, maybe you wanna to be a tester for a minute? Try this version (copy and replace)
obsidian_local_images_plus_latest.zip

[esodesod]

@esodesod Ok, maybe you wanna to be a tester for a minute? Try this version (copy and replace) obsidian_local_images_plus_latest.zip

Sure!

After replacing and reloading Obsidian, it seems to (still) throw the same TypeError from Local REST API > part of etag function, about the same part of the (compiled) code as before:

if (!isStats && typeof entity !== "string" && !Buffer.isBuffer(entity)) {
  throw new TypeError("argument entity must be string, Buffer, or fs.Stats");
}

[Sergei-Korneev]

Well, the buffer package was uninstalled. Probably it not the place. Keep on researching, guys ;)

[esodesod]

Well, the buffer package was uninstalled. Probably it not the place. Keep on researching, guys ;)

Quick update/question; I just did a quick diff on the compiled main.js from 0.15.8 (released) and the "latest" package you sent above (uninstalled buffer-version, if I understand correct); both files (main.js) seems to be identical (no diff); is that suppose to be, or could it be that buffer is not (actually) uninstalled (fully)?

Or to ask in another way; how can I verify buffer package is removed successfully? If I look in the 0.15.8 version, I see multiple references to e.g. "Buffer.isBuffer" (not seen in 0.15.7, which is working), but I guess you (and @57r31 ) understand this (way) better than me 🙈.

E.g. output from a quick vimgrep here:

To me, it (still) feels like @57r31 is into the right track, though I don't know how to address it (yet).

Please let me know if I can test anything else ❤️.

[esodesod]

Update (again 😅). I did a quick comparison on packages between 0.15.7 and 0.15.8. Removing the "jimp" library (using npm uninstall jimp) seems to be "working" (meaning the references to Buffer.isBuffer is removed from the 0.15.8, after running npm run build and doing a diff). Also works with Local REST API (verified).

I'm not "qualified" on how removing jimp library will impact Local Images Plus 0.15.8; but hopefully, you know best here @Sergei-Korneev, (when time permits, no rush ❤️).

[57r31]

yep, it was jimp and some other garbage using buffer, so uninstall won't help here

$ yarn why buffer
yarn why v1.22.19
[1/4] Why do we have the module "buffer"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "buffer@5.7.1"
info Reasons this module exists
   - "jimp#@jimp#custom#@jimp#core" depends on it
   - Hoisted from "jimp#@jimp#custom#@jimp#core#buffer"
info Number of shared dependencies: 2
=> Found "are-we-there-yet#buffer@6.0.3"
info Reasons this module exists
   - "npmlog#are-we-there-yet#readable-stream" depends on it
   - Hoisted from "npmlog#are-we-there-yet#readable-stream#buffer"
info Number of shared dependencies: 2

I think there must be a way to exclude module in rollup bundling stage

[Sergei-Korneev]

yep, it was jimp and some other garbage using buffer, so uninstall won't help here

Well, you can call this project a garbage here https://github.com/jimp-dev/jimp

Me on my part can assume that it is a big image processing library with its dependencies.
If it breaks your plugins guys ...uhm ...uhm I do not even know what to do

[57r31]

jimp is ok, I was talking about "npmlog#are-we-there-yet#readable-stream" that uses buffer too. Actually these two plugins is all I use - they perfectly add to each other, rest api for web clipper, and image downloader to make all links local. We should find a way to make them work together.

Actually jimp overwrites isBuffer method with its browser buffer implementation, so we can just overwrite it back.

[57r31]

@Sergei-Korneev please merge #48 @esodesod some testing? https://github.com/57r31/obsidian-local-images-plus/ rest api works now, and image download and converting seem to be working too

[esodesod]

@Sergei-Korneev please merge #48 @esodesod some testing? https://github.com/57r31/obsidian-local-images-plus/ rest api works now, and image download and converting seem to be working too

Nice @57r31!

I can confirm the "isBuffer fix for jimp" works for me as well; verified with both Local Images Plus 0.15.8 (with PR), and in combination with Local REST API 1.6.0 (does not throw isBuffer error anymore).

Guess this fixes https://github.com/coddingtonbear/obsidian-local-rest-api/issues/79 as well (and possibly more plugins, e.g. Telegram, mentioned here https://github.com/coddingtonbear/obsidian-local-rest-api/issues/79#issuecomment-1766841279)

[Sergei-Korneev]

Actually jimp overwrites isBuffer method with its browser buffer implementation, so we can just overwrite it back.

I just tried standard Buffer and with buffer package and got the same behavior

So this

looks like to me as a bullshit and crutch that may break at any moment and harm someone's ass. (no offense, prove me wrong)

So i am still curious why this part of code receives Uint8Array instead of Buffer

[57r31]

why this part of code receives Uint8Array instead of Buffer

coz there is no Buffer in browser, so ppl use browserified node version based on Uint8Array maybe in obsidian itself there is another version of Buffer

I agree that this looks like a crutch, but javascript is made of them. I tried something like "rollup-plugin-ignore" to simply remove buffer from bundle but it doesnt work as expected.

And unlikely that it breaks, all buffers are really Uint8Arrays

that's a shot from obsidian without any plugins at all

[Sergei-Korneev]

Yep :)

[57r31]

yep yep

do I really need to explain?

[Sergei-Korneev]

yep yep

do I really need to explain?

Yeeesss

this is your picture

It checks if it is a Buffer not an Unit8Array ¯_(ツ)_/¯

[57r31]

try this in obsidian with and without your plugin

[Sergei-Korneev]

try this in obsidian with and without your plugin

Maybe you should just open the source code of buffer package?

I'm beginning to think you're not sure what you're saying and instead of explaining, you're trying to get me to draw some conclusions so you don't look like a fool here.

Btw i still cannot see here "steps to reproduce section" etc. As I already said, I do not use those plugins.

And of courses you can fork this project and fix it as you wish. I will do it myself when I have time for this.

[Sergei-Korneev]

Well, I think we should do it at laest this way, to be sure it is some kind of Buffer, any objections?

//Jimp buffer package fix

Buffer.isBuffer = (e) => {

  return (
    e != null && (
    Object.getPrototypeOf(e) instanceof Uint8Array &&
    typeof e.constructor.isBuffer == "function" ) ||
    (e.isBuffer  || e._isBuffer)

    )

}

[Sergei-Korneev]

No? So I just published a new version. I am asking you for testing, dear users :)

[esodesod]

Again, no developer here, so I cannot verify the suggested code changes (or recommendations; maybe we can ask someone from the Obsidian Team or Dev-channel to have a quick look at the isBuffer part).

✅ I can however verify that Local Images Plus version 0.15.9 is working now in combination with Local REST API (1.6.0). 👍

[kissgyorgy]

I can however verify that Local Images Plus version 0.15.9 is working now in combination with Local REST API (1.6.0).

Doesn't work for me with same versions (0.15.9 and 1.6.0), I can confirm by disabling Local Images Plus, restarting Obsidian, Local REST API works, enabling it immediately breaks it.

[kissgyorgy]

The last release I see the intention was to remove the bufffer package by deleting it from package.json, but did not actually remove it from package-lock.json, see the diff here: https://github.com/Sergei-Korneev/obsidian-local-images-plus/commit/4b4ae126b3107bb9e0235401075fe183f12f7602#diff-053150b640a7ce75eff69d1a22cae7f0f94ad64ce9a855db544dda0929316519L792-R771

The current main: https://github.com/Sergei-Korneev/obsidian-local-images-plus/blob/3add519e4faee0df10043a5138c01447b8cab1e8/package-lock.json#L768-L771

It was only downgraded to 5.7.1.

[kissgyorgy]

I have a workaround for this by changing Buffer.from to new Uint8Array() and removing Jimp completely (so removing the convert to JPEG feature). This way Local REST plugin works correctly. For me, this is more than acceptable trade-off, I rather have full quality images locally. Feel free to use my fork: https://github.com/kissgyorgy/obsidian-local-images-plus

[esodesod]

For me, this is more than acceptable trade-off, I rather have full quality images locally.

I agree.

In my case, I've just pinned to version 0.15.7 (frozen version, using BRAT), before the pngToJpeg feature was added (0.15.8), and the added Jimp library and Buffer issues were introduced.

I guess the pngToJpeg is a nice feature; if you need that sort. Personally, I will not need that feature (which I guess leaves out the need for the whole Jimp library, including related dependencies, like Buffer). For my use case(s), a plugin making it possible to download images is the main feature I need (and I like this implementation), and (hopefully) being compatible with other plugins as well.

[kissgyorgy]

I just had an idea. What if you completely remove the resize feature and make a completely different plugin for only that functionality? The new "resize images" plugin would kick in whenever it finds an image of any kind in the vault. This way it's one less dependency, one less configuration but whoever needs it, can opt in.

[Sergei-Korneev]

@kissgyorgy @esodesod Hello guys, I didn't realize you were still struggling with a bug that was fixed almost a year ago.

Maybe you do not like the way it has been implemented, but at this point it supposedly works , no matter how.