Posted by digi24 on September 20, 2009 at 5:31pm
| Project: | Logging and alerts |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | minor |
| Assigned: | Dave Reid |
| Status: | closed (fixed) |
Issue Summary
Hi,
I do not know where this problem came from, but in the rare case that $log_msg['user']->uid and name are empty, there is an php error and no message gets send.
I guess uid should never be empty, but in case you have dlog turned off, errors might remain unnoticed. Took me a while find out. I will post a patch as soon as I have verified its working, have to wait for the error.
Index: logging_alerts/emaillog/emaillog.module
===================================================================
--- logging_alerts/emaillog/emaillog.module (revision 700)
+++ logging_alerts/emaillog/emaillog.module (working copy)
@@ -128,8 +128,8 @@
'@ip' => $log_msg['ip'],
'@request_uri' => $log_msg['request_uri'],
'@referer_uri' => $log_msg['referer'],
'@uid' => $log_msg['user']->uid,
'@name' => $log_msg['user']->name,
Comments
#1
Hmm, the uid part shouldn't cause any kind of error because the anonymous user's uid is 0. The name part might be undefined. Looking into this.
#2
Dave, I am quite sure that the problem with the empty object (and resulting php error) is not caused by your module. So all that I am proposing are two small if clauses that alternatively fill the field with some debug information. After all, its the modules purpose to handle situations when other modules fail.
I am just not so fluid with the objects, so I wanted to first observe that my clauses correctly handle the error that occurs only sporadically. Generally logging, also for cron and anonymous users works fine.
#3
If something is destroying the global $user (which is what this variable is), then something is going horribly, horribly wrong, because it always needs to be a proper user object, even when anonymous.
#4
Hello Dave,
you are right, the global user object should be intact, but you never know. In my case I experienced, that the global $user was set to scalar "0" (not the ->uid). I have included my modification to help other users debugging, I do not suggest it should be committed.
But where I had problems and what I think should be ammended is the way $log_msg['user']->uid and $log_msg['user']->name are accessed. As far as I understand, whenever $log_msg['user']->uid is destroyed by some other error, the logging module itself throws an error and does not email the original error message.
I am not too familiar with objects and classes, so feel free to close the issue in case I am writing total nonsense.
#5
Ok yeah, that's something going very wrong. Thank you for reporting #594956: broken logic in xmlsitemap_switch_user in another of my modules where the root problem was coming from. :)
#6
Won't fix here since this has been fixed in XML sitemap and if this pops up again, should be fixed at the source.
#7
Hello Dave,
sorry to bother you, but I would like to ask you to at least include the is_object check or similar.
The reason is simple:
It will not be typically noticed otherwise if such an error occurs. In my view this module should always send an email if an error occurs, even if its content might not be helpful or complete.
The whole point about the issue was that it did not "pop up".
Without testing, just Copy and paste from my patch above:
$uid=NULL;
$name=NULL;
if(is_object($log_msg['user'])){
if(isset($log_msg['user']->uid)){
$uid = $log_msg['user']->uid;
}
if(isset($log_msg['user']->name)){
$name = $log_msg['user']->name;
}
}
...
$message = t($message, array(
'@base_url' => $base_url,
'@severity' => $log_msg['severity'],
@@ -128,8 +152,8 @@
'@ip' => $log_msg['ip'],
'@request_uri' => $log_msg['request_uri'],
'@referer_uri' => $log_msg['referer'],
- '@uid' => $log_msg['user']->uid,
- '@name' => $log_msg['user']->name,
+ '@uid' => $uid,
+ '@name' => $name,
'@link' => strip_tags($log_msg['link']),
'@message' => strip_tags($msg),
));
#8
The error I experience is related: for anonymous users,
$log_msg['user']->namedoes not exist.$log_msg['user']is an object, and the uid property is present, just not the name property. The above check would help in my case. I hope it gets committed soon.#9
Fixed in 6.x-1.x-dev.
#10
When will there be a new 1.x release or 2.x release? There have been many commits since the last stable release.
#11
Just pushed 6.x-2.0 and 7.x-2.0 releases.