Has anyone else experienced this problem and can let me know if they have a workaround?
This started happening couple days ago and I haven’t been able to login to GTalk via Smal API (GMail logs me in fine).
The code that uses Smack API hasn’t changed in months.
The error first started appearing with an earlier version of the API, but I upgraded to 3.2.2 (just in case it’s something that was fixed recently) and problem still continues to happen.
JDK 1.6 on Win7x64
Here is what I see (repeated over and over for each attempt)
NFO: Trying to reconnect GoogleTalk attempt no. 1
SMACK VERSION: 3.2.2
java.lang.NullPointerException
at org.jivesoftware.smackx.provider.DelayInformationProvider.parseExtension(DelayInformationProvider.java:93)
at org.jivesoftware.smack.util.PacketParserUtils.parsePacketExtension(PacketParserUtils.java:768)
at org.jivesoftware.smack.util.PacketParserUtils.parsePresence(PacketParserUtils.java:248)
at org.jivesoftware.smack.PacketReader.parsePackets(PacketReader.java:232)
at org.jivesoftware.smack.PacketReader.access$000(PacketReader.java:43)
at org.jivesoftware.smack.PacketReader$1.run(PacketReader.java:70)
I would suspect then that something in a stanza from gtalk has changed which Smack can no longer parse correctly. In this case something specific to a Delayed Delivery extension. It is also possible that you simply weren’t getting an extension of this type before so you simply did not actually run this bit of code.
Try running it with debug on so you can see the content of the presence stanza you are receiving and post it back here.
I don’t know where you get that list of attributes from, as Flow has mentioned, we really need to see the raw stanza that came from the server.
Specifically, you should see a stanza with an element like this within it. This is what will trigger the provider that you are seeing the exception in.
I would guess that this was triggered by use case #3 of the Last Activity in Presence spec. Not sure if there is any other case which would embed a delay info element in a presence packet.
did you read my comment and look at the darn code?
This could have been addressed after the initial stock trace a month ago!
I got that list from inside the variables in the method I provided above. From parser variable. Thought that was plenty of information to debug this problem.
A 5 year old can see a problem with not checking for null after requesting a value from a map when they weren’t the ones who created the thing.
Do you understand that you are impacting others by inaction?
If you need full stanzas for ANY debug, then print the darn thing on any exception thrown!
and while at it, PLEASE fix the insane memory leak smack creates when connectivity is poor. There is a fix with a weak map or list someone offered in the forums - it’s killing me! Been around for years and no one is fixing it.
Smack is open source and supported by a hand of volunteers. If you want things to get fixed, try a least to be polite or search for a contracter to fix it for you if you are unable to do so. You are using a free of charge library that comes without any warranty. Patches are always welcome. I have a community repository, which includes some fixes, at github.
Spamming the log file with debug information would be bad pratice. If you want that someone fixes a problem for you, then you should at least be able to reproduce the problem and provide the related XMPP stanzas. Beeing rude and begging for fixes at the same time will not solve your problem.
Excuse me? In case you don’t realize it we are (sorry, were) trying to help.
The reason we want to see the stanza in question is to see if it is actually valid, as the stamp attribute is required for the element in question. The delay info element is pretty pointless without it. Obvously, to debug a parsing problem, the actual input would be nice to have.
Open source projects usually come with easier ways to submit changes.
And ways for people/moderators to do releases, which I didn’t see on ignite.
You clearly had to start your own repo to get fixes in… which, you gotta admit, is messed up.
As you can tell from my registration date, I’ve been writing for smack for a bit.
You have to understand the frustration.
Who do I send a patch to?
“Spamming the log” is exactly what you have to do on exception whenever you REQUIRE something for debug information. Otherwise exactly this sh1t happens! These are real time errors, you may not see them repeat enough to ever reproduce on demand. If you want stanzas - you have to provide them!
Sorry, man, but I haven’t seen you try and help till 5 min ago.and you had all the info from Sept 1!
I call bullsh1t!
“Helping” would have been to not leave a patronizing “you must be doing something wrong message” that takes 5 seconds on Sept 3, but going to look at the darn code that I gave you full trace on!
Helpful would have been to look at the code on 21st when I left you a full eval on the attributes on ‘parse’ on.
But HEY “f it, he wasn’t nice to my wasting his damn time” - is much faster to write then… looking at the code.
And “required” is supplied by humans at Google. They make mistakes too. Doesn’t mean the api needs to crash ungracefully and not even provide me with enough debug info as to what happened. Especially since this info is NOT crucial to a login.
You mean jive software. They have handed over smack, openfire etc to the community years ago and are not longer actively involved. I know that the current situation is not perfect when it comes to community driven development. There has been an inicative to move the source repositories to github, but then again there is not much manpower to do so. On the other hand you can simply send a patch to the forums to discuss it. If it’s considered valuable then a issue will be created and commited to the trunk. Plus github creates an strong temptation to just merge changes which can lead to slow but steadily code polution.
alex_1 wrote:
“Spamming the log” is exactly what you have to do on exception whenever you REQUIRE something for debug information. Otherwise exactly this sh1t happens! These are real time errors, you may not see them repeat enough to ever reproduce on demand. If you want stanzas - you have to provide them!
You can’t really fix an error you are unable to reproduce. We don’t want stanzas; you want stanzas to debug and solve the problem. So just enable smack debuging. Which is, for a good reason, disabled as default.
I hear your point about bloating code, but that’s what moderators do. You don’t need to allow newbs throwing code in. They can email/post fixes. Veterans can throw code in for them.
(no need to run the whole loop or the if statement if we never had the stampString.)
As you can see from reading the code, it was even written to recover from when stamp is optional (see the if (stamp==null) below in the method. It’s just it never happened before, so path never got executed that way.
I will reply, against my better judgement, as I am continually being insulted here. At the very least I guess I should defend myself.
On Sept. 3rd, I told you what my suspicions were and asked for more information, which you never gave until the 21st, and I didn’t read until the 24th (i.e. this morning). You may want to read the first reply again, as I, at no point said that “you must be doing something wrong”, in fact, I said that gtalk might be doing something wrong, which you have now confirmed with the stanza you sent.
I have no idea what prompted the “bullshit” remark. I have been trying to help you as soon as I saw that you have supplied more information that I asked for. Since I only saw your latest posting a couple of hours ago, I am not sure what you expect. I didn’t spend the weekend reviewing the ignite forums, and I don’t get paid to try to help people out via this forum, or to write code for Smack. I do it when I have extra time, most of which has been dedicated to Openfire in the last few months.
As for the failed login, I agree that having it fail like that is certainly undesirable, but the problem is not so simple as just parse it anyway. Parsing is not context aware, so the provider doesn’t know that the message is part of a login (or even a presence packet). It’s behaviour in this case, like most others is to simply fail when there is something wrong with the packet. Of course the attribute could simply be skipped in the provider, but that would then produce a DelayInfo class without a timestamp, which is also logically nonsensical.
I added my notes (copied from this thread) as to possible solution.
Hope this gets resolved quickly as I am sure I can’t be the only user of the API affected.
— Semi-related: —
All I am saying, is that had you actually looked at the source code the thread dump was pointing to, it would have been super clear as to what happened in less time than it took to think of possible failures. (Stanza didn’t uncover anything at this point).
I understand you like the ease of reading stanzas, but again: real time apps don’t always reliably produce the same behavior. You can’t just always expect them.
Clearly, it wasn’t me who was faking the Google’s stanzas just to waste everyone’s time. Assuming parser works ok (and by now - it better) - it was clear that the tag was indeed missing the param (as you can see from stanza that it does).
Look, man, I’ve been using this API from 2006 (as you can see from the date on profile). Clearly I am not one of those “hey, never read the docs, your stuff’s broke!” posts. Simple curtasy of looking at the stack trace is all that was implied in my going at ya.
I support other open source projects myself. And this is actually for one of them (JClaim). So I know what it’s like to have an open source project and support it.
And having a login bug is unacceptable for ANY api for this long. Regardless of who/how/why does the maintenance.
If they (ignite) didn’t give you guys a way to build a release or maintain it themselves in a timely manner using your feedback, then I agree with Flow that a new github repo is a proper approach.
That’s exactly what happened to a number of MSN/Yahoo IM open source projects.
This doesn’t seem to be a Smack problem. The other client is sending a malformed presence in form of a delayed element without the stamp attribute. A fix within DelayInformationProvider is IMHO not desirable. You should contact the developer of the other client and inform them that they send incorrect stanzas.
While I am waiting for the reply from the user as to what client he is using, lets take this idea for a spin:
So you are proposing (by not changing smack) that any one user in my user list can stop me from logging in? That sounds like a security issue and a reliability issue, rendering the entire Smack API unusable in any production environment.
And it all can be solved by 1 “if” statement.
Also, presense of the:
/*
if date could not be parsed but XML is valid, don’t shutdown
connection by throwing an exception instead set timestamp to current
I don’t see security affected, just reliability. But this problem is everywhere: If a user is able to send you a malformed stanza, Smack will always disconnect. This behavior is sharead along many other XMPP libraries/clients and even severs (e.g. openfire). It’s more or less “by design” because it’s nearly impossible to recover 100% when a malformened stanza is received: What action should you take? The other enity send you message that you didn’t understand, and now? What if the message told you to perform a certain action? So the “best” (IMHO) solution is to disconnect the connection, which also raises the awareness that something has gone wrong.
Adding the extra code statement for this corner case is exactly the code pollution I don’t like to see. Why sould a XMPP client take care of the errors other clients make? Wouldn’t it be better to fix the problem on the other side instead? Maybe it is possible to increase the login robustness by catching Exceptions at the right point in the login process. But then you are maybe not notified that something unforeseen has happened. I don’t like the idea either.
Oh and the code does not mean that it’s ok to send delayed delivery without an stamp attribute. The code comment clearly states that we set the current date only in case the stamp provided one could not be parsed. Which is IIRC there because the old XEP stamp format specification was ambiguous.