godbus / dbus

Native Go bindings for D-Bus
BSD 2-Clause "Simplified" License
956 stars 222 forks source link

server method call doesn't return if an error is being returned #244

Closed GaikwadPratik closed 2 years ago

GaikwadPratik commented 3 years ago

Hi,

I'm referencing server example to create a d-bus server. The entire code is

import (
    "errors"
    "fmt"

    "github.com/godbus/dbus"
    "github.com/godbus/dbus/introspect"
    "github.com/rs/zerolog/log"
)

const serviceName = "com.hiveio.vmmanager"
const objectPath = "/com/hiveio/vmmanager"
const intro = `
<node>
<interface name="com.hiveio.vm.Manager">
<method name="CheckHostForMigration">
<arg direction="in" type="s"/>
<arg direction="in" type="s"/>
<arg direction="out" type="b"/>
</method>
</interface>` + introspect.IntrospectDataString + `</node> `

func InitiateServer() error {
    conn, err := dbus.SystemBus()
    if err != nil {
        log.Error().Err(err).Msg("While initiating connection to system bus in initiateServer")
        return err
    }

    iface := VMManagerDbusInterface{}
    conn.Export(iface, dbus.ObjectPath(objectPath), serviceName)
    conn.Export(introspect.Introspectable(intro), dbus.ObjectPath(objectPath), fmt.Sprintf("%s.Introspectable", serviceName))

    reply, err := conn.RequestName(serviceName, dbus.NameFlagDoNotQueue)
    if err != nil {
        log.Error().Err(err).Stack().Msg("While checking name on system bus in initiateServer")
        return err
    }
    if reply != dbus.RequestNameReplyPrimaryOwner {
        log.Error().Err(err).Stack().Interface("reply", reply).Msg("Service name is already taken")
        return errors.New("name already taken")
    }
    log.Info().Msg(fmt.Sprintf("Listening on %s - %s ...", serviceName, objectPath))
    select {}
}

type VMManagerDbusInterface struct{}

func (server VMManagerDbusInterface) CheckHostForMigration(guestName string, cpuxml string) (bool, *dbus.Error) {
    log.Info().Str("guestName", guestName).Msg("Received request for checking host compatibility")
    var body []interface{}
    body = append(body, "TestErrorMessage", "TestErrorCode")
    e := dbus.NewError("TestErrorName", body)
    return true, e
}

I'm trying to test this code using linux terminal before creating a client in my main project just to make sure everything works. The invocation is done using:

root@hive1:~# dbus-send --system           \
  --dest=com.hiveio.vmmanager \
  --type=method_call          \
  --print-reply               \
  /com/hiveio/vmmanager       \
  com.hiveio.vmmanager.CheckHostForMigration \
  string:"TestGuest" string:"TestCPU"

If CheckHostForMigration does not return an error, meaning the second parameter as nil, the above call gets executed and returns value correctly. However, if there is an error being returned, then I get following error:

Error org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.

Can anyone please help? Another question is, what should be correct return type dbus.Error or dbus.DBusError?

invidian commented 2 years ago

I'm also affected by this issue.

guelfey commented 2 years ago

@GaikwadPratik the issue in this concrete case is that the error name is invalid - it's expected to be in interface.member notation, so for example com.hiveio.vmmanager.TestErrorName here. Otherwise the error reply is silently dropped at the moment, which is of course not nice. Of course some feedback would be nice, but there's really no good interface for it with the Export mechanism. Maybe just an additional log, though I normally try to keep direct logging from library code to an absolute minimum

GaikwadPratik commented 2 years ago

@guelfey ,

What is the difference between dbus.Error or dbus.DBusError? Which one should be preferred ?

guelfey commented 2 years ago

Both are possible. Error is basically the internal representation and simplest possible implementation of DBusError. The latter one is a bit nicer to use if you already have some type that implements error inside your main code; the first one is simpler otherwise if you just want to return a specific D-Bus error message.