🚀 go-pugleaf

"Darlington ADIBE" <bcb@triative.com> wrote in message
news:43864c38@newsgroups.borland.com...

> when a client calls ReadLn, and a timeout occures
> what should the client (my code) do

Disconnect and reconnect.  Unless you write your code to be able to recover
the communications from incomplete data without having to disconnect.

> should i clear the buffer cos when i send  another command
> and calls ReadLn, it comes back with the previous reply
> which off course makes my application not to function well.

If you are not going to disconnect, then clearing the buffer is absolutely
needed.  Just know that the sender will have no way of knowing that you are
doing so.  And if the sender also timed out, then there may be unsent data
that will still corrupt the communication anyway.

In general, reconnecting is best, though.  Keep in mind that Indy implements
its reading timeout on a per-byte basis, not a per-operation basis.  For a
reading timeout to occur in between two bytes, something serious likely
happened on the socket, or the connection is very very slow, and the
connection should be reset.  But that is your choice.

>   AnsiString SMS = StringReplace(SMSText, "\r\n", "\r", TReplaceFlags() <<
> rfReplaceAll);
>   cmd.sprintf("SENDSMS#%s#%s#", SMS, AnsiString(ToNumber));

That code should be moved inside the 'try' block.  COM methods are not
allowed to let uncaught exceptions escape.  Any AnsiString operation that
allocates new AnsiString values can potentially raise exceptions if there
are any memory problems.

>     if( Fetch(Reply , "#") == "SENDSMS") *pVal = WideString(Reply);

That code is dangerous.  You *MUST* release ownership of the BSTR that the
WideString contains.  Your code is not currently doing that, so when the
WideString goes out of scope, the BSTR is automatically freed, and the code
that is calling sendSMS() will receive an invalid BSTR pointer.  If you are
going to continue using WideString then you must call its Detach() method to
release ownership, ie:

    if( Fetch(Reply , "#") == "SENDSMS")
        *pVal = WideString(Reply).Detach(); // <-- here

>     else if(tcpp->ReadLnTimedOut)
>       *pVal = WideString("ERR:Remote server timedout");
>     else *pVal = WideString("ERR:Unknown remote server response");

Same here as well:

    else if( tcpp->ReadLnTimedOut )
        *pVal = WideString("ERR:Remote server timedout").Detach();
    else
        *pVal = WideString("ERR:Unknown remote server response").Detach();

>     *pVal = WideString("ERR:" + e.Message);

And here:

    *pVal = WideString("ERR:" + e.Message).Detach();

In fact, I do not recommend that your pVal parameter be used for reporting
error messages at all.  I strongly suggest that you make use of proper COM
error handling instead, which your code is not currently doing.

>     return S_FALSE; /*Error(e.Message.c_str(), IID_IesemesAPI);*/

Why are you not using Error()?  S_FALSE does not convey a failure, but your
exception handler did catch a failure and you are throwing away all of the
data for that failure.

I suggest you use the following code instead:

    STDMETHODIMP TesemesAPIImpl::sendSMS(BSTR SMSText, BSTR ToNumber,  BSTR*
pVal)
    {
        try
        {
            AnsiString SMS = StringReplace(SMSText, "\r\n", "\r",
TReplaceFlags() << rfReplaceAll);

            AnsiString cmd;
            cmd.sprintf("SENDSMS#%s#%s#", SMS,
AnsiString(ToNumber).c_str());

            tcpp->WriteLn(cmd);
            AnsiString Reply = tcpp->ReadLn(LF, IdTimeoutDefault, -1);

            if( Fetch(Reply , "#") != "SENDSMS")
            {
                // if you decide to disconnect on timeout, then...
                //
                // if( tcpp->ReadLnTimedOut )
                //    tcpp->DisconnectSocket();

                return Error(
                    tcpp->ReadLnTimedOut
                        ? "Remote server timed out",
                        : "Unknown remote server response",
                    IID_IesemesAPI,
                    E_FAIL);
            }

            *pVal = WideString(Reply).Detach();
        }
        catch(const Exception &e)
        {
            return Error(e.Message.c_str(), IID_IesemesAPI, E_UNEXPECTED);
        }
        return S_OK;
    }

Also, make sure that your TesemesAPIImpl class implements the
ISupportErrorInfo interface in order for the calling code to be able to
retreive the error data that Error() generates internally (including the
error message strings).  The easiest way to do that is to add
ISupportErrorInfoImpl to your class's ancestor list:

    class TesemesAPIImpl :
        //...
        public ISupportErrorInfoImpl<&IID_IesemesAPI>
    {
        //...
    };

> i commented the Error(e.Message.c_str(), IID_IesemesAPI); this
> because an error msg also popup when a timeout occures.

There is nothing in the code you showed that would do that.  For starters, a
reading timeout would not throw an exception in the first place, so your
exception handler would not be invoked to call Error().

> the server might have successfully executed the command, but just
> that the ACK back to the client did not arrive on time.

There is no way for the client to know if that happened.  If the ACK is
lost, the client has to assume that the command was not received and
executed.  The whole point of sending an ACK is for the client to know the
response to the command.  Without the ACK, the command could be in limbo for
all the client knows.  All the more reason to just reset the entire
connection when reading a reply to a command.

> When Error(e.Message.c_str(), IID_IesemesAPI);*/ is allowed to
> execute the client goes into unknow state

The only way that can happen is if the code that is calling sendSMS() is not
set up to handle COM errors properly in the first place.

> 2. (Mybe out off line) How best can one handle Exception in a COM
> (Automated Lib in an ActiveX Lib).

See the example above.


Gambit

Remy, thanx alot.

Your comments will surely help me write a better COM,. however from what
your have suggested it looks like i have to rethink my design in view of
both the server and the client.

To help you understand better let me walk you through my design:

The client software connects to the server by calling a COM method "Connect"
at this point only connection is established, the server will not proccess
any cmd from the client if the client is not Authenticated by the client app
calling a COM Authenticate method which then send the UserID and Password to
the server for athentication,

if authenticate pass:
the client is informed and the ThreadID is map to the UserID using a Memory
TStringList "Name=Value" this is how I         check for connections that
have been authenticated.

if authentication fail:
infor client and do nothing else

From above it mean that if i have to reconnect on readtimeout, then i also
have to reauthenticate as the ThreadID = UserID mapping will be invlaid,
since ThreadID changes for every connection "Already the server removes the
ThreadID = UserID mapping when a Thread disconnects"

The Server is actually use to send SMS to Mobile phone through a GSM
Gateway,
When it receives a "SENDSMS" cmd from an authenticated client (here is where
it get complicated)

it first try to send the SMS (this takes some time) at this point the client
is waiting for a status report
if all went well it will send back an "SENDSMS#OK#Msg_ID#" the msg_id is
used by both the client and the server for tracking the msg ie query if the
msg have be received by the receiptant or queqed for delivery by SMSC etc.
the server will then post billing info into a database agaist the UserID

if the operation failed, the server will send "SENDSMS#FAIL#reason#" and
that is all.

from above: if a timeout occures on the client while waiting for the status
report, the client have no way of knowning if the SMS was sent or not, and
it it resend it, it stand the risk of been bill twice for the same msg, even
if I use your idea of disconnecting and reconnecting on timeout the client
will still not know

So I seek for professional advice on this.

Darl.

> Also, make sure that your TesemesAPIImpl class implements the
> ISupportErrorInfo interface in order for the calling code to be able to
> retreive the error data that Error() generates internally (including the
> error message strings).  The easiest way to do that is to add
> ISupportErrorInfoImpl to your class's ancestor list:
>
>     class TesemesAPIImpl :
>         //...
>         public ISupportErrorInfoImpl<&IID_IesemesAPI>
>     {
>         //...
>     };
>

Don;t forget to add ISupportErrorInfo to the COM internface map as well:

BEGIN_COM_MAP(TesemesAPIImpl)
  ...
  COM_INTERFACE_ENTRY(ISupportErrorInfo)
END_COM_MAP()

Mark

"Remy Lebeau (TeamB)" <no.spam@no.spam.com> wrote in message
news:4386ba83$1@newsgroups.borland.com...

> I suggest you use the following code instead:

Using this code means that the calling application have to test the returned
String (*pVal) for NULL (or empty string), if it NULL or empty it mean then
that an error occured.

Can he retrive the error details and how?

For BCB programmer GetLastError() i assume, how about other platforms (VB)
do they have such facility?

"Darlington ADIBE" <bcb@triative.com> wrote in message
news:43878392@newsgroups.borland.com...

> Using this code means that the calling application have to test
> the returned String (*pVal) for NULL (or empty string), if it
> NULL or empty it mean then that an error occured.

No, it does not.  That is not how you are supposed to be handling COM errors
in the first place.  You need to check the HRESULT that the method actually
returns.  When everything is ok, the HRESULT is set to either S_OK or some
other positive number.  A negative number indicates an error.  That is how
COM determines whether an error actually occured or not.  For example:

    WideString result;
    if( pEesemesAPI->sendSMS(WideString("some text here"), WideString("some
number here"),  &result) >= 0 )
        // ok, use result as needed...
    else
        // failed, do something else...

Or, using standard COM macros:

    WideString result;
    if( SUCCEEDED(pEesemesAPI->sendSMS(WideString("some text here"),
WideString("some number here"),  &result)) )
        // ok, use result as needed...
    else
        // failed, do something else...

Or:

    WideString result;
    if( FAILED(pEesemesAPI->sendSMS(WideString("some text here"),
WideString("some number here"),  &result)) )
        // failed, do something else...
    else
        // ok, use result as needed...

In these examples, I am using the WideString class to manage all of the BSTR
memory management automatically.  Otherwise, you have to call
SysAllocString...() and SysFreeString() manually every time.

> Can he retrive the error details and how?

To get extended error information beyond the HRESULT value, he would have to
call COM's GetErrorInfo() function to retreive an IErrorInfo interface.  The
Error() method inside your COM object creates an IErrorInfo interface
internally that GetErrorInfo() can then retreive later.  For example:

    WideString result;
    HRESULT hRes = pEesemesAPI->sendSMS(WideString("some text here"),
WideString("some number here"),  &result);
    if( SUCCEEDED(hRes) )
        // ok, use result as needed...
    else
    {
        // failed, do something else...

        TComInterface<ISupportErrorInfo> Intf;
        if( SUCCEEDED(pEesemesAPI->QueryInterface(IID_ISupportErrorInfo,
(LPVOID*)&Intf)) )
        {
            if( Intf->InterfaceSupportsErrorInto(IID_IesemesAPI) == S_OK )
            {
                TComInterface<IErrorInfo> Err;
                if( GetErrorInfo(0, &Err) == S_OK )
                {
                    WideString ErrMsg;
                    Err->GetDescription(&ErrMsg);
                    // use ErrMsg as needed...
                }
                else
                    // no error info is available...
            }
        }
        else
            // use hRes as needed...
    }

> For BCB programmer GetLastError() i assume

No.  GetLastError() has no meaning in COM.  It is a Win32 API error
mechanism, not a COM one.

> how about other platforms (VB) do they have such facility?

VB uses GetErrorInfo() and the IErrorInfo interface.  However, VB is very
tightly integrated with COM, so a lot of these details are hidden from VB
users, as VB handles all of the error handling and error reporting
automatically.


Gambit

"Remy Lebeau (TeamB)" <no.spam@no.spam.com> wrote in message
news:438b485b$1@newsgroups.borland.com...

> No, it does not.  That is not how you are supposed to be handling COM
errors
> in the first place.  You need to check the HRESULT that the method
actually
> returns.  When everything is ok, the HRESULT is set to either S_OK or some
> other positive number.  A negative number indicates an error.  That is how
> COM determines whether an error actually occured or not.  For example:
>

This is my first COM base and Multi-thread Application please bear with me.

When I used the Type Lib Editor to add the method sendSMS to the COM
Interface, the editor added this two declearation to the file

 HRESULT         __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
ToNumber/*[in]*/,
                                     BSTR* pVal/*[out,retval]*/);
  BSTR            __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
ToNumber/*[in]*/);

My assumation was that the overload functions was for convicence, since by
standard, a COM Method can only return HRESULT, the second overload method
was add to call the origainal method and handle the return of the BSTR,
thereby allowing a programmer to used code like

WideString result = COMObj->sendSMS(WideString("test"),
WideString("number"));

which is more clean and readable than

WideString result;
if( COMObj->sendSMS(WideString("some text here"), WideString("some number
here"),  &result) >= 0 )

Also in VB the Method

 HRESULT         __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
ToNumber/*[in]*/,
                                     BSTR* pVal/*[out,retval]*/);
was not expose rather

  BSTR            __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
ToNumber/*[in]*/);

was expose using the same COM dll and on the same machine

"Darlington ADIBE" <bcb@triative.com> wrote in message
news:438b5b9d@newsgroups.borland.com...

> When I used the Type Lib Editor to add the method
> sendSMS to the COM Interface, the editor added
> this two declearation to the file

The first is the actual COM declaration.  The second is a Borland wrapper
that calls the first one, as well as performs basic error handling to throw
an exception when a COM error is reported.

> My assumation was that the overload functions was for convicence

That is correct.

> the second overload method was add to call the origainal method
> and handle the return of the BSTR, thereby allowing a programmer
> to used code like

Yes.  However, you have to be careful with such code.  WideString has some
quirky behavior that can cause memory leaks if you are not aware of how you
are using it.  In particular, WideString's BSTR constructor makes a copy of
the BSTR data but does not take ownership of the original BSTR, whereas
WideString's BSTR '=' operator does take ownership of the original BSTR.  To
ensure that the compiler does not accidently cause leaks to occur to your
code, you should explicitally separate the construction of a WideString from
its assignment when passing COM strings to the WideString, ie:

    WideString result;
    result = COMObj->sendSMS(WideString("test"), WideString("number"));

> which is more clean and readable than

Yes, it is more clean.  But you are not taking into account that the wrapper
code has the potential of throwing an exception.  So you should always make
sure that your code is exception-safe when using the COM object.

> Also in VB the Method
>
>  HRESULT         __fastcall sendSMS(BSTR SMSText/*[in]*/,
> BSTR ToNumber/*[in]*/, BSTR* pVal/*[out,retval]*/);
> was not expose

Yes, it is.  Non-Borland environments can only see what is actually in the
Type Library.  They never see the wrapper code at all.  Do not get confused
by the fact that VB has a similar syntax to what Borland's wrapper code
allows.  VB has its own built-in wrappers for COM operations.  Like I said
earlier, VB is very closely integrated with COM.  A lot of COM's
complexities are hidden to VB users.

For example, this VB code, which does not use the 'On Error' mechanism:

    Dim result
    result = COMObj.sendSMS("test", "number")

Is equivilent to this C++ code:

    WideString result;
    result = COMObj->sendSMS(WideString("test"), WideString("number"));

However, this VB code, which does use the 'On Error' mechanism:

    Dim result
    On Error Resume Next
    result = COMObj.sendSMS("test", "number")
    If Err.Number <> 0 Then MsgBox Err.Description
    // do something else...

Is equivilent to this C++ code:

    WideString result;
    if( FAILED(COMObj->sendSMS(WideString("test"), WideString("number"),
&result)) )
    {
        TComInterface<IErrorInfo> Err;
        WideString Desc;
        if( GetErrorInfo(0, &Err) == S_OK )
            Err->GetDescription(&Desc);
        ShowMessage(Desc);
    }
    // do something else...

Or this C++ wrapper code:

    WideString result;
    try
    {
        result = COMObj->sendSMS(WideString("test"), WideString("number"));
    }
    catch(const Exception &)
    {
        ShowMessage(E.Message);
    }
    // do something else...

>   BSTR            __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
> ToNumber/*[in]*/);
>
> was expose using the same COM dll and on the same machine

Wrong.  That method is not in the COM DLL at all.  It is only in Borland's
wrapper code that you end up including into your Application executable.


Gambit

"Remy Lebeau (TeamB)" <no.spam@no.spam.com> wrote in message
news:438b859f$1@newsgroups.borland.com...

> The first is the actual COM declaration.  The second is a Borland wrapper
> that calls the first one, as well as performs basic error handling to
throw
> an exception when a COM error is reported.

Then it will been more correct to alway call the second method and allow the
wrapper to do it basic function rather than bypass it by calling the first
(actual) declaration ie:

this

WideString result;
result = COMObj->sendSMS(WideString("test"),WideString("number"));

instand of this

WideString result;
if( COMObj->sendSMS(WideString("some text here"), WideString("some number
here"),  &result) >= 0 )


but off cousre within a try... catch().


> Yes.  However, you have to be careful with such code.  WideString has some
> quirky behavior that can cause memory leaks if you are not aware of how
you
> are using it.  In particular, WideString's BSTR constructor makes a copy
of
> the BSTR data but does not take ownership of the original BSTR, whereas
> WideString's BSTR '=' operator does take ownership of the original BSTR.
To
> ensure that the compiler does not accidently cause leaks to occur to your
> code, you should explicitally separate the construction of a WideString
from
> its assignment when passing COM strings to the WideString, ie:

What you are saying is that there is a difference between this

 WideString result;
 result = COMObj->sendSMS(WideString("test"), WideString("number"));

and this

 WideString result  = COMObj->sendSMS(WideString("test"),
WideString("number"));

and that the first is safer for  COM (thats a new one for me)

So far so good, all the above in from the COM user point of view, how about
from the COM developer point of view, which in this case is where i fall
within (Developer).

it more easier if am developing the COM to be used just within my own App.
but in this case my COM is targetted to third party developers.

is including the
public ISupportErrorInfoImpl<&IID_IesemesAPI>

and using
return Error(e.Message.c_str(), IID_IesemesAPI, E_UNEXPECTED);

to return Error good enough for my COM such that the third party developer
can use all the standard error handling routin you have mentioned so far
(for VB & C++).

Which of this method do i document for the COM user

this
 HRESULT         __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
ToNumber/*[in]*/,  BSTR* pVal/*[out,retval]*/);

or this
 BSTR            __fastcall sendSMS(BSTR SMSText/*[in]*/, BSTR
ToNumber/*[in]*/);

other commerical COM API alway document the second.

Thanx
Darl

"Darlington ADIBE" <bcb@triative.com> wrote in message
news:438c1166@newsgroups.borland.com...

> Then it will been more correct to alway call the second method
> and allow the wrapper to do it basic function rather than bypass
> it by calling the first (actual) declaration

There are times when the wrapper is not efficient or desired.  Having both
options available allows you to choose the best one for your actual code.

> What you are saying is that there is a difference between this
>
>  WideString result;
>  result = COMObj->sendSMS(WideString("test"), WideString("number"));
>
> and this
>
>  WideString result  = COMObj->sendSMS(WideString("test"),
> WideString("number"));

There can be, yes.  In the second one, there are two possibilities that can
occur due to , or lack of, optimizations during compiling:

1) a default-constructed WideString is created and then the '=' operator is
invoked.  This is the same as the first one further above.  No memory leak
occurs in this situation.

2) the call to the '=' operator is omitted and the BSTR from the COM object
is passed to WideString's BSTR constructor instead, ie:

    WideString result(COMObj->sendSMS(WideString("test"),
WideString("number")));

A memory leak occurs in this situation since the constructor does not take
ownership of the BSTR.

> it more easier if am developing the COM to be used just within
> my own App.  but in this case my COM is targetted to third
> party developers.

That is exactly what COM is good for.  BCB-created COM objects can work just
fine in other environments.  You just have to learn about the quirks in the
VCL and wrapper codes to make sure you do things properly.

> is including the
> public ISupportErrorInfoImpl<&IID_IesemesAPI>
>
> and using
> return Error(e.Message.c_str(), IID_IesemesAPI, E_UNEXPECTED);
>
> to return Error good enough for my COM such that the third
> party developer can use all the standard error handling routin
> you have mentioned so far (for VB & C++).

Non-Borland environments expect IErrorInfo to be used.  That is part of the
standard COM error handling architecture.

> Which of this method do i document for the COM user

The first one.  As I told you before, the second one is only Borland wrapper
code, and is not accessible to non-Borland environments at all.  They can
only use what is actually in the Type Library.


Gambit

"Remy Lebeau (TeamB)" <no.spam@no.spam.com> wrote in message
news:438c25f8$1@newsgroups.borland.com...

> The first one.  As I told you before, the second one is only Borland
wrapper
> code, and is not accessible to non-Borland environments at all.  They can
> only use what is actually in the Type Library.

Recheck this fact, the second declaration is what showed up in VB6 with my
COM and a Commerical COM i used, and the comerical documentation only
documented the BSTR return type declaration.

Please check while I also re-check on another Machine (just in case)

Darl