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

Various fixes #4

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Various fixes #4

wants to merge 6 commits into from

Conversation

bcotton
Copy link
Contributor

@bcotton bcotton commented Dec 31, 2022

  • Fixed cross-platform file path handling
  • Added ability to handle 301 and 302 redirects
  • small fix for permaLink and UTF8 characters

@erohtar
Copy link
Owner

erohtar commented Dec 31, 2022

Thank you so much for all these contributions, all of them look like great improvements. I'll go through them individually and merge these soon.
Quick Question - Does the pathing change break compatibility with existing path format (without ending slashes) in the settings file?

@bcotton
Copy link
Contributor Author

bcotton commented Dec 31, 2022

Does the pathing change break compatibility with existing path format (without ending slashes) in the settings file?

Unsure, I'm testing on a mac. My path does not have a trailing slash.

@erohtar
Copy link
Owner

erohtar commented Jan 2, 2023

I did various tests today, and I can confirm that the pathing change works well and is backward compatible.

But I did face a problem and it's 100% repeatable for me right now. This error happens each time when downloading .json

SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at createNotes (xxx\redditSaver.js:57:19)
    at IncomingMessage.<anonymous> (xxx\redditSaver\redditSaver.js:33:3)
    at IncomingMessage.emit (node:events:525:35)
    at endReadableNT (node:internal/streams/readable:1358:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)

I checked the saved.json and it's always truncated at the same spot
"awarders": [], "all_awardings": [], "locked": false, "author_flair_background_color": null, "collapsed_because_crowd_control": null, "mod_reports": [], "

The full saved.json shows this data right after the truncation
"awarders": [], "all_awardings": [], "locked": false, "author_flair_background_color": null, "collapsed_because_crowd_control": null, "mod_reports": [], "quarantine": false, "mod_note": null

Any idea what's causing the problem?

@bcotton
Copy link
Contributor Author

bcotton commented Jan 3, 2023

I'm having similar problems, but it will succeed about 25% of the time. It's something to do with the timing of the Promise, dunno.

Still looking into it.

@erohtar
Copy link
Owner

erohtar commented Jan 3, 2023

For me it was doing that 100% of the time yesterday at one point. Since then I have unsaved the latest posts/comments and saved some new ones and I can't recreate it right now - I wish I didn't make a change so I could keep looking into it, but my current json download doesn't give me an error.
I'll make a release with most of your changes, except the promise and redirect ones - I figure the issue has to be in those somewhere.

@bcotton
Copy link
Contributor Author

bcotton commented Jan 5, 2023

I pushed a new commit. This works for me, every time.

@erohtar
Copy link
Owner

erohtar commented Jan 5, 2023

Thank you for the update - pls allow me a couple days to use/test it and I'll merge with the main branch.

@erohtar
Copy link
Owner

erohtar commented Jan 10, 2023

I have been using your updated version for last few days and have seen no issues whatsoever. Just created a new release.
Thank you again for all the improvements.

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

Successfully merging this pull request may close these issues.

2 participants