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

Fix error "Object of class DateTime could not be converted to int" #377

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rbharlam
Copy link

Hi,

First of all, thank you very much for the great work!

We're using Icinga Web 2 in combination with the nagvis plugin. However, this combination causes the following exception when performing various actions like placing hosts/services in nagvis:

{"type":"error","message":"Object of class DateTime could not be converted to int","title":"Error: Unhandled Exception"}

The only other related reference I was able to find is this: https://community.icinga.com/t/icingadb-web-1-1-0-and-nagvis-module/12713

This very small adjustment fixes the problem for us completely.

@miken32
Copy link

miken32 commented Jun 18, 2024

I just updated to 8.2 yesterday and came here to report this. The problem is in /opt/nagvis/share/server/core/classes/objects/NagVisStatefulObject.php on line 315, but I'm sure there are more incompatibilities to be found.

(Editing to add I thought I was commenting on an issue not a PR. Obviously you know where the error is lol)

@miken32
Copy link

miken32 commented Jun 18, 2024

I don't think the value is ever anything but a datetime. The function could be rewritten as:

    /**
     * Returns state timestamp as human readable date
     */
    public function get_date(string $attr): string {
        if($this->state[$attr] ?? null instanceof \DateTime) {
            self::$dateFormat ??= cfg('global','dateformat');

            return $this->state[$attr]->format(self::$dateFormat);
        }

        return 'N/A';
    }

But I think they are supporting versions of PHP from like 2015 so your fix probably will be more acceptable.

@rbharlam
Copy link
Author

@miken32 very lucky finding a PR 10 minutes after creation for the same problem 😄 Especially because as far as I can tell this exists for some time already.

Could have created a bug instead too, but might as well help with a less time-intensive solution since the devs already did so much work :)

I can adjust the function if necessary, just let me know which is the preferred solution.

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.

None yet

2 participants