NetMQ / Samples

Samples of NetMQ
88 stars 43 forks source link

ClientAsync Ctor Overload Missing Call to Default Ctor #2

Closed Camuvingian closed 7 years ago

Camuvingian commented 7 years ago

In project MajordomoProtocol file MDPClientAsync the overloaded ctor

public MDPClientAsync([NotNull] string brokerAddress, string identity)
{
    if (string.IsNullOrWhiteSpace(brokerAddress))
        throw new ArgumentNullException(nameof(brokerAddress), "The broker address must not be null, empty or whitespace!");

    if (!string.IsNullOrWhiteSpace(identity))
        m_identity = Encoding.UTF8.GetBytes(identity);

    m_brokerAddress = brokerAddress;
}

is not calling this().

drewnoakes commented 7 years ago

Well spotted. Fancy making a pull request?

Camuvingian commented 7 years ago

Okay, I will do this tonight.

Camuvingian commented 7 years ago

There is also a miss-match in the way replies are handled for MDPClient vs MDPClientAsync. In the ProcessReceiveReady method we have in MDPClientAsync

private void OnProcessReceiveReady(object sender, NetMQSocketEventArgs e)
{
    Exception exception = null;
    NetMQMessage reply = null;
    try
    {
        reply = m_client.ReceiveMultipartMessage();
        if (ReferenceEquals(reply, null))
            throw new ApplicationException("Unexpected behavior");

        m_timer.EnableAndReset(); // reset timeout timer because a message was received

        Log($"[CLIENT INFO] received the reply {reply}");

        ExtractFrames(reply);
    }
    catch (Exception ex)
    {
        exception = ex;
    }
    finally
    {
        ReturnReply(reply, exception);
    }
}

private void ExtractFrames(NetMQMessage reply)
{
    if (reply.FrameCount < 4)
        throw new ApplicationException("[CLIENT ERROR] received a malformed reply");

    var emptyFrame = reply.Pop();
    if (emptyFrame != NetMQFrame.Empty)
    {
        throw new ApplicationException($"[CLIENT ERROR] received a malformed reply expected empty frame instead of: { emptyFrame } ");
    }
    var header = reply.Pop(); // [MDPHeader] <- [service name][reply] OR ['mmi.service'][return code]

    if (header.ConvertToString() != m_mdpClient)
        throw new ApplicationException($"[CLIENT INFO] MDP Version mismatch: {header}");

    var service = reply.Pop(); // [service name or 'mmi.service'] <- [reply] OR [return code]

    if (service.ConvertToString() != m_serviceName)
        throw new ApplicationException($"[CLIENT INFO] answered by wrong service: {service.ConvertToString()}");
}

Then in MDPClient

private void ProcessReceiveReady (object sender, NetMQSocketEventArgs e)
{
    // a message is available within the timeout period
    var reply = m_client.ReceiveMultipartMessage ();

    Log ($"\n[CLIENT INFO] received the reply {reply}\n");

    // in production code malformed messages should be handled smarter
    if (reply.FrameCount < 3)
        throw new ApplicationException ("[CLIENT ERROR] received a malformed reply");

    var header = reply.Pop (); // [MDPHeader] <- [service name][reply] OR ['mmi.service'][return code]

    if (header.ConvertToString () != m_mdpClient)
        throw new ApplicationException ($"[CLIENT INFO] MDP Version mismatch: {header}");

    var service = reply.Pop (); // [service name or 'mmi.service'] <- [reply] OR [return code]

    if (service.ConvertToString () != m_serviceName)
        throw new ApplicationException ($"[CLIENT INFO] answered by wrong service: {service.ConvertToString()}");
    // now set the value for the reply of the send method!
    m_reply = reply;        // [reply] OR [return code]
}

These should be the same, or have I missed something?

somdoron commented 7 years ago

I'm not sure, less familiar with this code sample. Anyway, if you think something is wrong, fix it and send a PR. If you are wrong, someday someone else will send a PR to fix that.

Camuvingian commented 7 years ago

Okay, fair enough. I am undergoing a process of creating a new type of transport mechanism based on Majordomo, if I find any further anomalies I will send a PR.