-
-
Notifications
You must be signed in to change notification settings - Fork 128
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
Fix sharedfiles scraping error when Steam returns non-English page, fix rejected request detection and improve type detection #316
base: master
Are you sure you want to change the base?
Conversation
This PR now also improves type detection because I don't want to get on your nerves with another PR. Some Artworks can have only one breadcrumb (for some reason) so this change looks at the first breadcrumb as a backup if the second one is missing. I have also added an if check to hopefully suppress further errors and instead just leave type at |
You prob want to merge it into #230 |
This PR improves an existing feature in version 3.x.x, so I chose master |
Oh okay, I didn't know you were aware that v4 was being worked on |
Yeah, targeting master is appropriate for fixes to an existing feature, until v4 is out. I'm sorry about sorta ghosting this, been pretty busy for the past while. I'll try to deal with some open stuff, including this, later today. |
if (body.success) { | ||
callback(null); | ||
} else { | ||
callback(new Error(body.error)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this tested and confirmed working? Typically, when Steam returns a success
key, it's an eresult and not a bool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, Steam returns the following here: { success: false, error: 'There was a problem deleting this comment. Please try again.' }
if (body.success) { | ||
callback(null); | ||
} else { | ||
callback(new Error(body.error)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also tested; Steam returns the following: { success: false, error: 'There was a problem posting your comment. Please try again.' }
Hey, sorry for bumping, but it would be cool if you could review this PR as it fixes bugs present in the |
Hey! I've gotten multiple bug reports that scraping the Posted date of a sharedfile can cause an error as Steam can apparently serve a non-English page as response to our request.
I didn't consider this to be a possibility and didn't encounter this issue while testing, even though my accounts are also not English but German.
I have added a
&l=english
query parameter to the request and added a fallback to the Posted readout, so that even if it should fail again, no error will be thrown and the Posted value remainsnull
.This fix has been tested and the users who reported it confirmed that the issue did not reoccur afterwards.