Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS S3 Synchronisation Error on iOS deivce #4810

Closed
zxkmm opened this issue Apr 4, 2021 · 11 comments · Fixed by #5212
Closed

AWS S3 Synchronisation Error on iOS deivce #4810

zxkmm opened this issue Apr 4, 2021 · 11 comments · Fixed by #5212
Labels
bug It's a bug

Comments

@zxkmm
Copy link

zxkmm commented Apr 4, 2021

Environment

Joplin version: 10.7.2 (Latest on iOS for now)
Platform: iOS
OS specifics: 14.4.1 on iPhone (Latest for now)

Steps to reproduce

  1. First time synchronisation on windows with E2EE enable
  2. Synchronisation to AWS S3 Server
  3. First time synchronisation on iOS
  4. got an error like this:
    “XMLParserError:Non-whitespace before first tag.
    Line: 0
    Column: 1
    Char: S”。

Describe what you expected to happen

The notes seems fine in the first time that I synchronisation on iOS (This Error message appear when the synchronisation is almost finished) , but when this error appear, the synchronisation function is totally broken and noting can be synchronisation neither other platform---->iOS nor iOS-----> other platform.

The notes that I have, almost all notes is I manually added, and a few is I import form md files that I exported from Notion app.

I tried 10 times, even change the provider of the server and it can be resocced every time.

Logfile

IMG_0782
IMG_0783
IMG_0784
IMG_0785

@zxkmm zxkmm added the bug It's a bug label Apr 4, 2021
@zxkmm zxkmm changed the title AWS S3 Synchronisation Error in iOS deivce AWS S3 Synchronisation Error on iOS deivce Apr 5, 2021
@thisdotrob
Copy link
Contributor

Also experiencing this!

@stale
Copy link

stale bot commented Jun 2, 2021

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may comment on the issue and I will leave it open. Thank you for your contributions.

@stale stale bot added stale An issue that hasn't been active for a while... and removed stale An issue that hasn't been active for a while... labels Jun 2, 2021
@leematos
Copy link
Contributor

leematos commented Jul 7, 2021

Switching from WebDav to S3 today I hit this.

I've narrowed it down to files that were edited on iOS causing the problem. If you only create new files, and delete files on iOS sync will work, but once you edit a file it will throw the error.

I've spent a few hours trying to debug it but still no dice. An error that seems to point to the root of the problem: Leonidas-from-XIV/node-xml2js#345 (it seems like the problem is related to XML format parsing.)

The problem i'm struggling with now is trying to figure out is it bombing when attempting to read the edited file on the local device, or when it tries to send the file over the wire.

[Tue Jul 06 2021 21:53:05.320]  LOG      21:53:05: FileApi: list 
[Tue Jul 06 2021 21:53:05.694]  LOG      21:53:05: FileApi: put locks/sync_mobile_64d5337700534b76a403fb516bc0a15b.json null
[Tue Jul 06 2021 21:53:05.697]  LOG      WE S3 PUTTING!
[Tue Jul 06 2021 21:53:05.764]  LOG      21:53:05: FileApi: list 
[Tue Jul 06 2021 21:53:05.766]  LOG      21:53:05: FileApi: list 
[Tue Jul 06 2021 21:53:05.897]  LOG      21:53:05: FileApi: stat 08c7cca23822480bb863a61caa3cbff5.md

@laurent22 do you know on iOS which FileAPI drivers are being used? it doesn't appear to be packages/app-mobile/util/fs-driver-rn.js nor lib/file-api-driver-memory.js

I was able to generate that WE S3 PUTTING! message from lib/file-api-driver-amazon-s3.js but it doesn't appear to be using that driver's stat (based on the lack of my error messaging coming up in logs.)

@leematos
Copy link
Contributor

leematos commented Jul 7, 2021

I just restarted my test env, and am now seeing more error logging so inching closer. Lemme look again tomorrow with fresh eyes!

@zxkmm
Copy link
Author

zxkmm commented Jul 7, 2021

I just restarted my test env, and am now seeing more error logging so inching closer. Lemme look again tomorrow with fresh eyes!

Thank you for your contribution! To attract the attention, I suggest you to start a new issue and post what you have found, thank you again!

@leematos
Copy link
Contributor

leematos commented Jul 7, 2021

@zxkmm I'm gonna use this thread for now until I have a pull request to hopefully close this one out for good :D

I have pinpointed the source of the error:

async s3HeadObject(key) {
return new Promise((resolve, reject) => {
this.api().headObject({
Bucket: this.s3_bucket_,
Key: key,
}, (err, response) => {
if (err) reject(err);
else resolve(response);
});
});
}

For some reason, this can sometimes returns an error, but also, a response:

[Wed Jul 07 2021 13:36:18.259]  LOG      HEAD OBJ KEY: e982802dfe294c199a9e3249e50d6e97.md
[Wed Jul 07 2021 13:36:18.906]  LOG      [XMLParserError: Non-whitespace before first tag.
Line: 0
Column: 1
Char: N]
[Wed Jul 07 2021 13:36:18.907]  LOG      RESPONSE: {"AcceptRanges": "bytes", "ContentLength": 658, "ContentType": "application/octet-stream", "ETag": "\"c15cea6cfd0d2ff73e42910e5e2996ef\"", "LastModified": 2021-07-07T17:35:40.000Z, "Metadata": {}}

So the code rejects, even though we actually have the response we expect. i hacked around this in my simulator by ignoring the error for now and syncing in the simulator works.

I think upgrading the aws-sdk to https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md#2723 will fix this, and I'm hoping to try that later today!

EDIT: that version is way older than the current pinned version, so still trying to figure out a way to properly work around this.

@leematos
Copy link
Contributor

leematos commented Jul 8, 2021

The plot continues to thicken.

I now have a state where my joplin simulator generates an error attempting to read the head for this file above, but an abstracted test case using the same logic outside of joplin never generates an error. I'm also running the latest 2.94 branch of the aws-sdk. I'm thinking it's something related to aws-react-native and an assumption it's making.

My gameplan is to try two things:

  1. Attempt the new v3 AWS api, and see if that fixes it.
  2. if not, to write some logic that if the error is XMLParseError & the response is valid json, to parse the response and ignore the error.

@leematos
Copy link
Contributor

leematos commented Jul 9, 2021

I have a branch built that just replaces the head s3 api call with the aws-sdk-v3-js library, and it works perfectly!

I'm going to submit a pull request for the fix but we'll need Laurent's input to decide if we want to replace all of the s3 endpoints (now) or just this one, and migrate over time.

@zxkmm
Copy link
Author

zxkmm commented Jul 9, 2021

I have a branch built that just replaces the head s3 api call with the aws-sdk-v3-js library, and it works perfectly!

I'm going to submit a pull request for the fix but we'll need Laurent's input to decide if we want to replace all of the s3 endpoints (now) or just this one, and migrate over time.

Wow that’s amazing! Thank you so much!

@spacemonkey
Copy link

Did this get backed out, and is there another effort to fix? Still cannot sync from iOS via S3 driver.

@leematos
Copy link
Contributor

@spacemonkey -- There is a new MR now -- some merge conflicts but looking to get it sorted out. (I'm still affected by this as the MR creator so I'm motivated to fix 😛) #5312

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants