pubkey / rxdb

A fast, local first, reactive Database for JavaScript Applications https://rxdb.info/
https://rxdb.info/
Apache License 2.0
21.15k stars 1.03k forks source link

Unable to store blob type attachments in Electron #3022

Closed Wuchv closed 3 years ago

Wuchv commented 3 years ago

Case

bug

Issue

I've got an error when trying to store an attachment in electron renderer process, with pouchdb adapter.

    file: Pick<File, 'name' | 'type'> & { blob: Blob }

    await putAttachment(
              {
                id: file.name,
                data: file.blob,
                type: file.type,
              },
              true
            );

Throws:

Uncaught (in promise) TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string 
or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of Blob

Versions

    "pouchdb-adapter-leveldb": "^7.2.2",
    "rxdb": "^9.12.1",
    "electron": "^11.1.1",

Info

Code

After I see the source code, problem in this method, Buffer.from can't accept a Blob parameter.

  createBlobBuffer: function createBlobBuffer(data, type) {
    var blobBuffer;

    if (_util__WEBPACK_IMPORTED_MODULE_3__.isElectronRenderer) {
      // if we are inside of electron-renderer, always use the node-buffer
      return Buffer.from(data, {
        type: type
      });
    }
  /* ... */

And it is impossible to encrypt the blob type attachments, JSON.stringify also can't accept a Blob parameter.

var _encryptValue = function _encryptValue(value) {
  return encrypt(JSON.stringify(value), this.password);
};
pubkey commented 3 years ago

Can you extend this test to reproduce the error?

Wuchv commented 3 years ago

electron test I can't download the file rxdb-local.tgz , may be because of the network reasons, so I didn't run the code, but I think this will reproduce the error.

kukagg commented 3 years ago

Can confirm that this is true. What was the reason to use node-buffer when isElectron()?

kukagg commented 3 years ago

I don’t think there’s any downside in removing L55-L60: https://github.com/pubkey/rxdb/blob/86967dc951c3e3e4e0a36b352dde5f272ad384b5/src/plugins/attachments.ts#L55-L60

Correct me if I’m wrong.

pubkey commented 3 years ago

I am not sure why I had to add this. We can remove everything as long as the tests still work. But first we need to reproduce your bug in the CI.

pubkey commented 3 years ago

Ah I've noticed that the electron tests are currently commented out I will try to reenable them.

Wuchv commented 3 years ago

I don’t think there’s any downside in removing L55-L60:

https://github.com/pubkey/rxdb/blob/86967dc951c3e3e4e0a36b352dde5f272ad384b5/src/plugins/attachments.ts#L55-L60

Correct me if I’m wrong.

I tried to delete this code and then the render process gone. Maybe because this or that

pubkey commented 3 years ago

I fixed the electron tests in the CI. Please make a PR with a reproduction of your problem

Wuchv commented 3 years ago

I fixed the electron tests in the CI. Please make a PR with a reproduction of your problem

npm WARN tarball tarball data for rxdb@file:rxdb-local.tgz (null) seems to be corrupted. Trying again. I still can't download the file rxdb-local.tgz.

pubkey commented 3 years ago

rxdb-local.tgz is generated locally in the ci. It is not downloaded.

pubkey commented 3 years ago

I had to disable the electron tests from the CI again. They still randomly fail. We should upgrade the electron versions in the examples and then we might be able to reproduce this. Please help if you have time.

Wuchv commented 3 years ago

I had to disable the electron tests from the CI again. They still randomly fail. We should upgrade the electron versions in the examples and then we might be able to reproduce this. Please help if you have time.

I reproduced this error here, the test code is in preload.js

pubkey commented 3 years ago

That helps a bit. But I need a PR with a reproduction in the CI, I spend too much time in the past to debug other peoples setup.

kukagg commented 3 years ago

Added this https://github.com/pubkey/rxdb/pull/3098 but no issue in the test env. Maybe there is something bigger at play or my test is totally off. @Wuchv Would be nice to have you take a look at it.

pubkey commented 3 years ago

@kuka we also have tests that run inside of electron https://github.com/pubkey/rxdb/tree/master/examples/electron/test But they are disabled in the CI atm because we first have to update electron otherwise it randomly fails.

ash0x0 commented 3 years ago

Same issue here. Electron 11.4, renderer process, IndexedDB adapter. Is there a current workaround or ideas for something to try? I tried putAttachment with Blob and ArrayBuffer. I'm thinking of trying base64 string but even if it works it's a pain to handle and temporary solution at best. Anything I can do to help resolve this?

pubkey commented 3 years ago

I am not sure what is happening here. Everyone has the same problem but noone wants to even update the electron example.

kukagg commented 3 years ago

@pubkey played with the electron test and it worked for me. I ran it many times and minor delay for async worked.

kukagg commented 3 years ago

Updated the PR with the Electron error reproduction and some improvements for the example https://github.com/pubkey/rxdb/pull/3098.

ash0x0 commented 3 years ago

I'm not sure what the process is but I'm ready to help with whatever. #3098 is a good repro for my use too.

kukagg commented 3 years ago

@ash0x0 I don’t believe there is any process. Just introducing a PR with a fix in it is 90% of the task. No permission needed.

Btw, I tested out omitting blob handler in here https://github.com/pubkey/rxdb/blob/7bbc78c37bc3fa579aed50c8636734bb335a84a2/src/util.ts#L419-L424 and it beared no fruit. I’ll play with it more but any help from you @ash0x0 and @Wuchv would be helpful.

@pubkey thank you for all the work you did/do!

pubkey commented 3 years ago

@ash0x0

ash0x0 commented 3 years ago

I'm not sure here what's done and not by @kuka ? it seems #3115 fixes the first item. Do I PR to enable tests? or does #3115 need more work?

kukagg commented 3 years ago

@ash0x0 I simply reproduced the error we’re all experiencing in electron test. The changes include improvements for the example as well. Now we have to fix the error itself. It’s unclear what the source of the issue is it seems. You could do what @pubkey laid out and then try to find out what the root of the issue is.

Items are:

Basically overarching goal is to make sure Electron test works as expected so that we can clearly see it’s initially failing and passing once the fix is introduced @ash0x0 .

I’ll be able to dig deeper sometime this/next week as well.

ash0x0 commented 3 years ago

Ok I'll get on 3 and 4 until you guys figure out the CI issue.

pubkey commented 3 years ago

It is very likely that updating the deps will fix the ci problems

pubkey commented 3 years ago

Closing this since it seems noone wants to work on it. Ping me when you want to work on that so I can reopen.

ash0x0 commented 3 years ago

@pubkey I am working on it, just not fixed. I did the version update, added #3098 and tested, the CI works on my fork, the issue is there and attachment fails so that's where I'm at. It's definitely src/util.ts#L408 and there are variations with electron and node version for buffer handling.

pubkey commented 3 years ago

Oh sorry. I reopened it.

pubkey commented 3 years ago

There is a good chance that this is solved in the new RxDB major version. https://github.com/pubkey/rxdb/issues/3279

Please test.

pubkey commented 3 years ago

Closing this, likely fixed in 10.0.0. Otherwise please make a PR with a test that fails in the CI, I spend enough time trying to figure out what is going wrong here.

Elendiar commented 3 years ago

On "electron": "^13.1.9" and "rxdb": "^10.0.3" cant store attachement in renderer.

Simple code like

await document.putAttachment({
          id: "cat.jpg",
          data: "foo bar asldfkjalkdsfj",
          type: "text/plain",
        });

throw this error: Uncaught (in promise) TypeError: Failed to execute 'readAsText' on 'FileReader': parameter 1 is not of type 'Blob'.

If i pass new Blob([string]) instead "foo bar...", i recieve

 Uncaught (in promise) TypeError: Failed to execute 'readAsArrayBuffer' on 'FileReader': parameter 1 is not of type 'Blob'.
    at Object.readAsArrayBuffer (index-browser.js?f480:108)
    at appendBlob (index-browser.js?bfb3:29)
    at loadNextChunk (index-browser.js?bfb3:72)
    at Object.binaryMd5 (index-browser.js?bfb3:75)
    at preprocessBlob (index.js?ac47:223)
    at preprocessAttachment (index.js?ac47:250)
    at eval (index.js?ac47:282)
    at Array.forEach (<anonymous>)
    at Object.preprocessAttachments (index.js?ac47:263)
    at idbBulkDocs (index.js?6a7b:282)

It happens, because function readAsArrayBuffer(blob, callback) ... in node_modules\pouchdb-binary-utils\lib\index-browser.js recieve blob as object. If wrap blob inside this func to new Blob([blob]) all works. Im not good in blobs, but how i can pass/transform data in electron renderer to make this work ?

UPD: I fix this behaviour with

attachments: {
    encrypted: false,
  },

in schema. Seems problem appears after encryption, without encryption all works fine. But works only with blob, not string. Stringdata still throw TypeError: Failed to execute 'readAsText' on 'FileReader': parameter 1 is not of type 'Blob'. in

return new Promise(function (res) {
      // browsers
      var reader = new FileReader();
      reader.addEventListener("loadend", function (e) {
        var text = e.target.result;
        res(text);
      });
      var blobBufferType = Object.prototype.toString.call(blobBuffer);
      /**
       * in the electron-renderer we have a typed array insteaf of a blob
       * so we have to transform it.
       * @link https://github.com/pubkey/rxdb/issues/1371
       */

      if (blobBufferType === "[object Uint8Array]") {
        blobBuffer = new Blob([blobBuffer]);
      }

      reader.readAsText(blobBuffer);  // Uncaught (in promise) TypeError: Failed to execute 'readAsText' on 'FileReader': parameter 1 is not of type 'Blob'.
    });
HZSamir commented 2 years ago

Hello, Setting encrypted to false did not fix this issue in my case. Thing is the same code used to work fine on rxdb 9, upgrading to 10 raised this issue. Anything I might try? Thank you for your time.

pubkey commented 2 years ago

:) https://github.com/pubkey/rxdb/issues/3022#issuecomment-832695350 Thank you for your time :)

HZSamir commented 2 years ago

Must've missed that, apologies. As always, thank you for you time.