search

jMonkeyEngine / SoftwareStore

BSD 3-Clause "New" or "Revised" License
8 stars 2 forks source link

Third party api used to generate user avatars responds with 403. Probably shouldn't be used or handled gracefully. #8

Open riccardobl opened 3 years ago

riccardobl commented 3 years ago

Following this issue: https://github.com/jMonkeyEngine/SoftwareStore/issues/6 The code in question is:

https://github.com/jMonkeyEngine/SoftwareStore/blob/master/src/main/java/com/jayfella/website/core/ImageDownloader.java#L15-L38

The api sometimes returns 403. I applied a workaround that returns an empty image in this case. A better approach should be used instead and the issue that causes the 403 error (rate limits?) be discovered. Alternatively this thing could be removed, since it is useless.

jayfella commented 3 years ago

At a guess, since this is a new issue since you moved servers and added several layers to the traffic route, it’s probably as a result of that.

I’d also argue the “since it’s useless” case that it’s actually very useful. All modern software creates a default avatar. The knock-on effects of removing this would return all non-avatars as 404 responses unless you took the time to alleviate that issue too.

I would personally inspect why the server is returning a 403. I would speculate it’s as a result of one or more of your traffic layers putting you in a shared environment.

riccardobl commented 3 years ago

The layers you are talking about do not exist. I speculate this was a temporary limitation on our ip range, or maybe an issue in their endpoints. But anyway the point here is that a failed avatar resolution shouldn't cause the registration to fail, and a static avatar can be used for users that do not have one.

jayfella commented 3 years ago

Those layers do exist. Docker, CloudFlare, and any other buzzword that's been implemented that routes the traffic through it.

Regardless, A static avatar "works", but the point of a unique avatar generator is so that even if the user does not upload an avatar, they are still uniquely identifiable by their pre-generated avatar. You may deem it un-necessary, but I find it hard to argue with every single modern piece of software that does the same. You may not realize it but you rely on this feature every time you browse through the jmonkey hub, identifying people by their avatar.

riccardobl commented 3 years ago

You may not realize it but you rely on this feature every time you browse through the jmonkey hub, identifying people by their avatar.

I know that very well, i still deem that unnecessary, and this "problem" is already solved by gravatar, that is the other avatar resolution service used in the code.

Those layers do exist. Docker, CloudFlare, and any other buzzword that's been implemented that routes the traffic through it.

Plase don't get sidetracked by FUD.

jayfella commented 3 years ago

When it was hosted and maintained under my provision it didn’t have this issue. Under your stack it does. It isn’t FUD. It’s reasonable cause.

Gravatar doesn’t create unique avatars. It pulls the avatar for the assigned email, or on failure, a gravatar logo. Again, the uniquety of the avatars is what is deemed important. I have no interest in pointless debates. I just ask that you don’t rip important areas of code through lack of understanding. I get that “you” don’t understand it’s significance, but “you” are not the audience. It’s there for a reason. If you lack the understanding, leave it alone.

oxplay2 commented 3 years ago

leave it alone.

so you say "i didnt provided everything, so it dont work, but leave it alone and let site dont work correctly"

interesting. edit: will not even say...

if you have something important to say, it should be help, not "leave BUG alone"

riccardobl commented 3 years ago

When it was hosted and maintained under my provision it didn’t have this issue. Under your stack it does. It isn’t FUD. It’s reasonable cause.

You are playing guessing game with random unrelated things. This issue is about the failed avatar resolution causing the registration to fail. The thirdparty service can be unreachable for multiple reasons, such as outages on the services, since it is used for a trivial matter (avatar resolution) this shouldn't cause the application to fail. What was the cause of the 403 error (that seems to have been temporary) is unknown but most likely unrelated to what you have proposed, and i repeat, a failure in a third party service shouldn't prevent the application from working as expected. The workaround i deployed fixes this problem but ,due to a time constraint, not in a graceful way. This issue is here to remember us that the problem is not solved.

Gravatar doesn’t create unique avatars.

Gravatar does generated unique avatars based on the hashed email when the identicon option is enabled.

I just ask that you don’t rip important areas of code through lack of understanding. I get that “you” don’t understand it’s significance, but “you” are not the audience. It’s there for a reason. If you lack the understanding, leave it alone.

The issue is here to be discussed, I don't care how it will be resolved ultimately. However my personal opinion is that what you are asking makes objectively no sense.

jayfella commented 3 years ago

I’m quite sure I am helping. Rule #1 when finding a bug that wasn’t previously there: what changed. I pointed out what changed.

I am also helping in retaining features, not ripping them out because “you don’t think it’s important” despite every single online software product doing the same.

Requiring the user to sign up to gravatar is pushing the problem away. Just solve the 403 that didn’t exist. That’s the issue.

riccardobl commented 3 years ago

I am annoyed by this. You didn't understand or read anything of what i said.

jayfella commented 3 years ago

I did and I disagree. But do what you will. I’ve spent too much time on something so simple already.

❤️ to JME.