Use TIdHttp inside TThread anonymous function doesn't raise exceptions

[rlebeau]

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.

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?

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.

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.

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);

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());
    }
}