Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Use TIdHttp inside TThread anonymous function doesn't raise exceptions
#1
Hi all,

I am using a subclass of TIdHttp to expose DoRequest function. I use it in a tool similar to Rest Debugger so what I want to achieve regardless of the code because I can't show it. But I am pretty sure you will get the idea.

I use this new class inside a thread so I can handle files downloads and uploads with progress bars correctly without blocking the UI, and I do all the necessary synchronizations with the UI. My only problem is client side exceptions for example when I do a request and the server is not running nothing happens at all. Nothing is shown in the memo or as dialog message. I understand that this behaviour is because I am executing the request inside a thread. 

I want to catch any exception that was raised on the server with error code 500 inside a TMemo as long as any response message, and any exception that is not related to server side internal errors to be displayed normally in a dialog message box.

now this is not the actual code but it is similar:


Code:
__fastcall TIdHttpEx::TIdHttpEx(TComponent *Owner)
    : TIdHTTP(Owner)
{
HTTPOptions = TIdHTTPOptions() >> hoWantProtocolErrorContent;
FHTTPBody.reset(new TIdHTTPBody);
FHTTPBody->IdHTTP = this;

FHasErrorResponse = false;
FRaiseExceptionOn500 = true;
FLastErrorResponse = "";
}
//---------------------------------------------------------------------------
int __fastcall TIdHttpEx::DoExecute(String AMethod, String AURL, TStream* AResponseContent, TStream* ARequestContent)
{
try
    {
    FHasErrorResponse = false;
    FLastErrorResponse = "";

    if (FHTTPBody->RequestStream == nullptr && ARequestContent != nullptr)
        FHTTPBody->RequestStream = ARequestContent;

    if (FHTTPBody->ResponseStream == nullptr && AResponseContent != nullptr)
        FHTTPBody->ResponseStream = AResponseContent;

    if (FHTTPBody->ResponseStream == nullptr && AResponseContent == nullptr)
        FHTTPBody->ResponseStream = new TMemoryStream;

    DoRequest( AMethod, AURL, FHTTPBody->RequestStream, FHTTPBody->ResponseStream,nullptr,-1);

    FResponseOk = (ResponseCode == 200);

    if (FHTTPBody->ResponseStream != nullptr)
        FHTTPBody->ResponseStream->Position = 0;
    }
catch (const EIdHTTPProtocolException &E)
    {
    if (FRaiseExceptionOn500)
        throw Exception(E.ErrorMessage);
    else
        {
        FLastErrorResponse = E.ErrorMessage;
        FHasErrorResponse = (FLastErrorResponse.Trim() != "");
        DoHTTPErrorEvent(FLastErrorResponse);
        }
    }
catch (const Exception &E)
    {
    ShowException(&E, nullptr); //this do the trick but I don't know if it is the right thing to do
    }
}
Reply
#2
(09-02-2020, 01:20 AM)Ahmed Sayed Wrote: I use this new class inside a thread so I can handle files downloads and uploads with progress bars correctly without blocking the UI, and I do all the necessary synchronizations with the UI. My only problem is client side exceptions for example when I do a request and the server is not running nothing happens at all. Nothing is shown in the memo or as dialog message. I understand that this behaviour is because I am executing the request inside a thread. 

There is no problem using TIdHTTP in a thread. So the problem has to be in how you are using it. But you did not show all of your code.

(09-02-2020, 01:20 AM)Ahmed Sayed Wrote: I want to catch any exception that was raised on the server with error code 500 inside a TMemo as long as any response message

Where is the code that delegates a server exception to the TMemo? I see you throwing your own Exception if a server error is caught, but I do not see any code that catches that Exception to display it in the TMemo. What does that try..catch (if there is one) look like? What does your OnHTTPErrorEvent handler (if there is one) look like?

(09-02-2020, 01:20 AM)Ahmed Sayed Wrote: and any exception that is not related to server side internal errors to be displayed normally in a dialog message box.

I would not suggest your component call ShowException() directly. You should expose that Exception info to the caller and let it decide how it wants to display it.

(09-02-2020, 01:20 AM)Ahmed Sayed Wrote:
Code:
HTTPOptions = TIdHTTPOptions() >> hoWantProtocolErrorContent;

I suspect you meant to use the << operator instead. The >> operator is REMOVING the hoWantProtocolErrorContent flag, which is not enabled by default to begin with. Based on what you have shown so far, you probably meant to use this instead:

Code:
HTTPOptions = HTTPOptions << hoNoProtocolErrorException << hoWantProtocolErrorContent;

Enabling the hoWantProtocolErrorContent flag takes effect ONLY if either:
  • the hoNoProtocolErrorException flag is also enabled.
  • the ResponseCode that is actually received is specified in the AIgnoreReplies parameter of TIdHTTP.DoRequest().

Neither of which are true in your example.

Note that by enabling the hoNoProtocolErrorException flag, there will be no EIdHTTPProtocolException thrown at all, so you can then remove that catch from your code.

(09-02-2020, 01:20 AM)Ahmed Sayed Wrote:
Code:
FResponseOk = (ResponseCode == 200);

200 is not the only ResponseCode that indicates success. At the very least, you should be accounting for ALL possible 2xx response codes instead, eg:

Code:
FResponseOk = ((ResponseCode / 100) == 2);

(09-02-2020, 01:20 AM)Ahmed Sayed Wrote:
Code:
catch (const EIdHTTPProtocolException &E)
{
if (FRaiseExceptionOn500)
throw Exception(E.ErrorMessage);

You are not checking the ResponseCode for 500 before throwing. When the hoNoProtocolErrorException flag is disabled, TIdHTTP throws EIdHTTPProtocolException for ANY non-2xx HTTP response that is not handled internally.

If you do throw, I would suggest NOT throwing a new exception, as you will lose some information from the original. Just re-throw the original instead:

Code:
catch (const EIdHTTPProtocolException &E)
{
if (FRaiseExceptionOn500 && ((ResponseCode / 100) == 5))
  throw; // <-- note the LACK of an exception object specified!

Or, at least wrap the caught exception in an outer Exception so you can capture the original in the thrown exception's InnerException property, should the receiver want to use that info (such as its ErrorCode and ErrorMessage properties) to understand WHY your manual exception was thrown in the first place, eg:

Code:
catch (const EIdHTTPProtocolException &E)
{
    if (FRaiseExceptionOn500 && ((ResponseCode / 100) == 5))
        IndyRaiseOuterException(new Exception("Server error"));

Although, if you enable the hoNoProtocolErrorException flag, then you would have to throw your own Exception, if that is what you really want.

So, I guess, depending on your needs, I would suggest either this implementation:

Code:
__fastcall TIdHttpEx::TIdHttpEx(TComponent *Owner)
    : TIdHTTP(Owner)
{
    HTTPOptions = HTTPOptions << hoNoProtocolErrorException << hoWantProtocolErrorContent;

    FHTTPBody.reset(new TIdHTTPBody);
    FHTTPBody->IdHTTP = this;

    FHasErrorResponse = false;
    FRaiseExceptionOn500 = true;
    FLastErrorResponse = "";
}

int __fastcall TIdHttpEx::DoExecute(String AMethod, String AURL, TStream* AResponseContent, TStream* ARequestContent)
{
    try
    {
        FHasErrorResponse = false;
        FLastErrorResponse = "";

        if (!FHTTPBody->RequestStream)
            FHTTPBody->RequestStream = ARequestContent;

        if (!FHTTPBody->ResponseStream)
            FHTTPBody->ResponseStream = (AResponseContent) ? AResponseContent : new TMemoryStream;

        DoRequest( AMethod, AURL, FHTTPBody->RequestStream, FHTTPBody->ResponseStream, nullptr, -1);

        if (FHTTPBody->ResponseStream)
            FHTTPBody->ResponseStream->Position = 0;

        FResponseOk = ((ResponseCode / 100) == 2);
        if (!FResponseOk)
        {
            if (FHTTPBody->ResponseStream)
            {
                FLastErrorResponse = ReadStringAsCharset(FHTTPBody->ResponseStream, Response->CharSet);
                FHasErrorResponse = (FLastErrorResponse.Trim() != "");
                FHTTPBody->ResponseStream->Position = 0;
            }

            if (FRaiseExceptionOn500 && ((ResponseCode / 100) == 5))
                throw Exception("Server Error"); // or whatever you want...

            DoHTTPErrorEvent(FLastErrorResponse);
        }
    }
    catch (const Exception &E)
    {
        // consider making a new event for this instead...
        ShowException(&E, ExceptAddr());
    }
}

Or this implementation:

Code:
__fastcall TIdHttpEx::TIdHttpEx(TComponent *Owner)
    : TIdHTTP(Owner)
{
    FHTTPBody.reset(new TIdHTTPBody);
    FHTTPBody->IdHTTP = this;

    FHasErrorResponse = false;
    FRaiseExceptionOn500 = true;
    FLastErrorResponse = "";
}

int __fastcall TIdHttpEx::DoExecute(String AMethod, String AURL, TStream* AResponseContent, TStream* ARequestContent)
{
    try
    {
        FHasErrorResponse = false;
        FLastErrorResponse = "";

        if (!FHTTPBody->RequestStream)
            FHTTPBody->RequestStream = ARequestContent;

        if (!FHTTPBody->ResponseStream)
            FHTTPBody->ResponseStream = (AResponseContent) ? AResponseContent : new TMemoryStream;

        try
        {
            DoRequest( AMethod, AURL, FHTTPBody->RequestStream, FHTTPBody->ResponseStream, nullptr, -1);
        }
        __finally
        {
            if (FHTTPBody->ResponseStream)
                FHTTPBody->ResponseStream->Position = 0;
        }

        FResponseOk = true;
    }
    catch (const EIdHTTPProtocolException &E)
    {
        FResponseOk = false;
        FLastErrorResponse = E.ErrorMessage;
        FHasErrorResponse = (FLastErrorResponse.Trim() != "");

        if (FRaiseExceptionOn500 && ((ResponseCode / 100) == 5))
            throw Exception("Server Error"); // or whatever you want...

        DoHTTPErrorEvent(FLastErrorResponse);
    }
    catch (const Exception &E)
    {
        // consider making a new event for this instead...
        ShowException(&E, ExceptAddr());
    }
}

Reply
#3
Thanks Remy for the reply,

Yes What I want is to simulate the same thing that is being done in for instance TRestClient/TRestRequest where I get any response from the server in the received content it self without the need to catch any exceptions.
The DoHTTPErrorEvent is where I get the catched error and pass it to the memo in the main program.

As I said before this is similar to REST debugger but with Indy TidHTTP instead. So here is the whole code of the class:
I made a little change after I posted my question but I did not made any changes based on your suggestions yet.

Header


Code:
//New Events
typedef void __fastcall (__closure *THTTPErrorEvent)(UnicodeString ErrorMessage);
typedef void __fastcall (__closure *TCriticalErrorEvent)(System::Sysutils::Exception* E);
//---------------------------------------------------------------------------
class TIdHttpEx : public TIdHTTP
{
    typedef TIdHTTP inherited;

private:
//Fields
unique_ptr<TIdHTTPBody> FHTTPBody;
bool FResponseOk;
bool FHasErrorResponse;
bool FRaiseExceptionOn500;
String FLastErrorResponse;

//Events
THTTPErrorEvent FOnHTTPErrorEvent;
TCriticalErrorEvent FOnCriticalErrorEvent;

//---------------------------------------------------------------------------
protected:
virtual String __fastcall GetFullURL();

//Methods
virtual int __fastcall DoExecute(String AMethod, String AURL, TStream* AResponseContent = nullptr, TStream* ARequestContent = nullptr);

//Do Events Methods
virtual void __fastcall DoHTTPErrorEvent(String ErrorMessage);
virtual void __fastcall DoCriticalErrorEvent(System::Sysutils::Exception* E);

//---------------------------------------------------------------------------
public:
//Fields/Properties
__property unique_ptr<TIdHTTPBody> Body = {read = FHTTPBody};
__property bool ResponseOk = {read = FResponseOk};
__property String FullURL = {read = GetFullURL};

__property bool HasErrorResponse = {read = FHasErrorResponse};
__property String LastErrorResponse = {read = FLastErrorResponse};
__property bool RaiseExceptionOn500 = {read = FRaiseExceptionOn500, write = FRaiseExceptionOn500};

//Constructor / Destructor.
__fastcall TIdHttpEx(TComponent *Owner);
virtual __fastcall ~TIdHttpEx(void);

//Methods
virtual int __fastcall Execute(String AMethod, String AURL, TStream* AResponseContent, TStream* ARequestContent = nullptr);
virtual int __fastcall Execute(String AMethod, String AURL, TStrings* AResponseContent, TStrings* ARequestContent = nullptr);
virtual int __fastcall Execute(String AMethod, String AURL, String AResponseContent, String ARequestContent = "");
virtual int __fastcall Execute(String AMethod, String AURL, TJSONValue* AResponseContent, TJSONValue* ARequestContent = nullptr);
virtual int __fastcall Execute(String AMethod, String AURL);
virtual void __fastcall Clear();

//Events
__property THTTPErrorEvent OnHTTPErrorEvent = {read = FOnHTTPErrorEvent, write = FOnHTTPErrorEvent};
__property TCriticalErrorEvent OnCriticalErrorEvent = {read = FOnCriticalErrorEvent, write = FOnCriticalErrorEvent};

//---------------------------------------------------------------------------
};
//---------------------------------------------------------------------------


Code File
Code:
//---------------------------------------------------------------------------

#pragma hdrstop
#include "IdHttpEx.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
//---------------------------------------------------------------------------
__fastcall TIdHttpEx::TIdHttpEx(TComponent *Owner)
    : TIdHTTP(Owner)
{
HTTPOptions = TIdHTTPOptions() >> hoWantProtocolErrorContent;
FHTTPBody.reset(new TIdHTTPBody);
FHTTPBody->IdHTTP = this;

FHasErrorResponse = false;
FRaiseExceptionOn500 = true;
FLastErrorResponse = "";
}
//---------------------------------------------------------------------------
__fastcall TIdHttpEx::~TIdHttpEx(void)
{
}
//---------------------------------------------------------------------------
int __fastcall TIdHttpEx::DoExecute(String AMethod, String AURL, TStream* AResponseContent, TStream* ARequestContent)
{
try
    {
    FHasErrorResponse = false;
    FLastErrorResponse = "";

    if (FHTTPBody->RequestStream == nullptr && ARequestContent != nullptr)
        FHTTPBody->RequestStream = ARequestContent;

    if (FHTTPBody->ResponseStream == nullptr && AResponseContent != nullptr)
        FHTTPBody->ResponseStream = AResponseContent;

    if (FHTTPBody->ResponseStream == nullptr && AResponseContent == nullptr)
        FHTTPBody->ResponseStream = new TMemoryStream;

    DoRequest( AMethod, AURL, FHTTPBody->RequestStream, FHTTPBody->ResponseStream,nullptr,-1);

    FResponseOk = (ResponseCode == 200);

    if (FHTTPBody->ResponseStream != nullptr)
        FHTTPBody->ResponseStream->Position = 0;
    }
catch (const EIdHTTPProtocolException &E)
    {
    if (FRaiseExceptionOn500)
        throw Exception(E.ErrorMessage);
    else
        {
        FLastErrorResponse = E.ErrorMessage;
        FHasErrorResponse = (FLastErrorResponse.Trim() != "");
        DoHTTPErrorEvent(FLastErrorResponse);
        }
    }
catch (Exception &E)
    {
    DoCriticalErrorEvent(&E);
//    Application->ShowException(&E);
//    ShowException(&E, nullptr);
    }

return ResponseCode;
}
//---------------------------------------------------------------------------
/*Important Methods*/
int __fastcall TIdHttpEx::Execute(String AMethod, String AURL, TStream* AResponseContent, TStream* ARequestContent)
{
return DoExecute(AMethod, AURL, ARequestContent, AResponseContent);
}
//---------------------------------------------------------------------------
int __fastcall TIdHttpEx::Execute(String AMethod, String AURL, TStrings* AResponseContent, TStrings* ARequestContent)
{
FHTTPBody->RequestStrings = ARequestContent;
int code = DoExecute(AMethod, AURL);
ARequestContent = FHTTPBody->ResponseStrings;
return code;
}
//---------------------------------------------------------------------------
int __fastcall TIdHttpEx::Execute(String AMethod, String AURL, String AResponseContent, String ARequestContent)
{
FHTTPBody->RequestText = ARequestContent;
int code = DoExecute(AMethod, AURL);
ARequestContent = FHTTPBody->ResponseText;
return code;
}
//---------------------------------------------------------------------------
int __fastcall TIdHttpEx::Execute(String AMethod, String AURL, TJSONValue* AResponseContent, TJSONValue* ARequestContent)
{
FHTTPBody->RequestJSON = ARequestContent;
int code = DoExecute(AMethod, AURL);
ARequestContent = FHTTPBody->ResponseJSON;
return code;
}
//---------------------------------------------------------------------------
int __fastcall TIdHttpEx::Execute(String AMethod, String AURL)
{
return DoExecute(AMethod, AURL);
}
//---------------------------------------------------------------------------
void __fastcall TIdHttpEx::Clear()
{
FHTTPBody->Clear();
Request->Clear();
Response->Clear();
}
//---------------------------------------------------------------------------
String __fastcall TIdHttpEx::GetFullURL()
{
return URL->URI;
}
//---------------------------------------------------------------------------
/*Do Events Methods Defs*/
void __fastcall TIdHttpEx::DoHTTPErrorEvent(String ErrorMessage)
{
if (FOnHTTPErrorEvent)
    FOnHTTPErrorEvent(ErrorMessage);
}
//---------------------------------------------------------------------------
void __fastcall TIdHttpEx::DoCriticalErrorEvent(System::Sysutils::Exception* E)
{
if (FOnCriticalErrorEvent)
    FOnCriticalErrorEvent(E);
}
//---------------------------------------------------------------------------
Reply
#4
(09-02-2020, 06:58 PM)Ahmed Sayed Wrote: Yes What I want is to simulate the same thing that is being done in for instance TRestClient/TRestRequest where I get any response from the server in the received content it self without the need to catch any exceptions.

Well, I already showed you the 2 ways to accomplish that with TIdHTTP.

(09-02-2020, 06:58 PM)Ahmed Sayed Wrote: The DoHTTPErrorEvent is where I get the catched error and pass it to the memo in the main program.

You still have not shown what your synchronization code actually looks like. You say you are not seeing anything in your TMemo, but you haven't shown HOW you are actually using TIdHTTPEx and trying to get its data into the TMemo. I can't help you without seeing that code.

Reply
#5
Sorry, maybe I explained it wrong.

I do catch server internal errors inside the TMemo fine, but for example when I send a request to a server that is not running, supposedly I should get an error "connection refused" because the client can't connect to the server in the first place. Now I if understand this correctly that type of errors are not internal server error and should be caught outside the thread executing the request. Now what I was trying to achieve is to make sure that all returned content from the server whether it is an error or not must be displayed in Memo, all other exception must be caught in a normal Message box.

Now just to understand this correctly If I want to make TIdHTTP behave the same way as TRestRequest I must enable both hoWantProtocolErrorContent ,hoNoProtocolErrorException So that content is passed to Response ContentStream. Right? Then any other error will be caught outside the thread in the CriticalErrorEvent.

Am I right or not?
Reply
#6
(09-03-2020, 04:00 AM)Ahmed Sayed Wrote: when I send a request to a server that is not running, supposedly I should get an error "connection refused" because the client can't connect to the server in the first place.

That is one possible error, yes.  There are other possible socket errors.  But yes, all non-HTTP errors would be handled by your outer catch (Exception &E) block (which BTW, should be catching by const reference, just as you are doing when catching EIdHTTPProtocolException).

(09-03-2020, 04:00 AM)Ahmed Sayed Wrote: Now I if understand this correctly that type of errors are not internal server error and should be caught outside the thread executing the request.

They will be thrown in the same thread that is executing the HTTP request.  You would have to catch and delegate them to other threads as needed.

(09-03-2020, 04:00 AM)Ahmed Sayed Wrote: Now what I was trying to achieve is to make sure that all returned content from the server whether it is an error or not must be displayed in Memo, all other exception must be caught in a normal Message box.

And that is fine, and the code I have given you should be doing exactly that.

(09-03-2020, 04:00 AM)Ahmed Sayed Wrote: Now just to understand this correctly If I want to make TIdHTTP behave the same way as TRestRequest I must enable both hoWantProtocolErrorContent, hoNoProtocolErrorException So that content is passed to Response ContentStream. Right?

That is one way to do it, yes.  The other way is to catch EIdHTTPProtocolException and copy its ErrorMessage text into your target TStream.

Reply
#7
Thanks, Remy for the help I really appreciate it
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)