Lukas,
I'm hoping that I've been using your excellent library long enough now to
occasionally contribute to the effort. I think there are a number of areas
in which I could help, such as debugging and English translation. (I've had
some problems with some of the language and terms used; and I suspect that
it might be even harder for those developers who don't know as much
English.)
I'm responding to this thread (hopefully) because of some issues about, and
suggestions for error handling. I've come across a number of situations
where error information is actually accurately captured by Synapse, but then
is hidden, redefined or lost as the code unwinds back up the stack. And this
is a pretty serious situation (for us at least).
Let's start with the case that Werner Hauptfleisch brought up. It's a good
example of several problems, including: error message language, and losing
information when backing out from an error (from trying to "squeeze"
multiple error message types into one set of error return values).
That SSL Lib problem hit me a few times, and I did not remember from one
episode to the next what "SSL is not implemented" means. It could mean so
many things! At which point it doesn't mean much at all. Though at least in
this situation we know it's an SSL problem. There are times when you get the
error message "Network subsystem is unusable" for *exactly* the same error.
[See below for an example.] This is also an error condition that will likely
hit every NEW user of Synapse, and possibly a few of us "seasoned" users
again... Choosing another way of handling it will mean less support needed
on the mailing list, happier users, and possibly *more* users...
---------
As there are a number of issues lurking in here, ranging from very simple to
somewhat complex architectural judgments, I'll start with the simple ones,
and put together a demo project to explore the rest. I'll probably stick
with Werner's SSL lib case for this entire message. I'm doing this to the
best of my current ability; I've studied these parts of the code a fair bit,
but I may well be mis-understanding things here and there. I welcome any
criticisms or corrections.
My first and simplest suggestion is that the text of that particular message
be changed to something more definitive and accurate... Possibly something
like "No SSL library is USED in (linked into) this program." I think that
would alert most programmers to the actual source of the error much faster
than "Network subsystem is unusable" (socket error 10091, which is actually
meant for completely different errors) or "SSL is not implemented";
hopefully others will offer even better phrases.
In ADDITION, I suggest using a technique we've used for a long time: This
error is part of a class of errors that are discovered at run time, but
which are uniquely PROGRAMMER (setup) ERRORS. We, not the user, installer,
etc., were the ones making the mistake, and this was predictable from the
original library code (as you indicated with your ReturnError() method,
called from nearly all the base class TCustomSSL methods). In those
situations, I call a function called, strangely enough <g>, ProgrammerError!
that does only one thing, throw an exception RIGHT AT THE POINT OF THE
PROBLEM:
procedure ProgrammerError(Msg : string);
begin
raise Exception.create('PROGRAMMER ERROR: ' + Msg);
end;
This makes that class of errors immediately recognizable to us developers,
and further narrows the possibilities we have to consider when encountering
the message "No SSL library USED..."
Further, I would suggest that the ReturnError() method of base TCustomSSL
class *directly* raise such an error (instead of passing back an error value
to the caller), NOT ONLY because of the general reason given above (to
direct attention exactly at the problem, which should only happen to
developers (when they're testing SSL)), but in this case because IF THE
ERROR is not raised as an exception here, it "gets lost in translation"
going back up the stack! or across object boundaries... (This is probably
due to the growth that Synapse experienced over the years.)
To clarify, let's look at an actual case (which also shows some other
problems)... First let's throw an example program together. Start a new
project with one button and a Memo; and drop this code in the unit:
(this
one done for Windows, and using Synapse Release # 37)
unit SSLlibErrorDemoU;
interface
uses
Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
Dialogs, StdCtrls;
type
TForm1 = class(TForm)
Button1: TButton;
Memo1: TMemo;
procedure Button1Click(Sender: TObject);
private
procedure writeln(s : string = '');
public
{ Public declarations }
end;
var
Form1: TForm1;
implementation
uses httpsend {, ssl_openssl};
{$R *.dfm}
procedure TForm1.writeln(s: string);
begin
Memo1.Lines.Add(s);
end;
procedure TForm1.Button1Click(Sender: TObject);
var // This routine is a minor modification of the HttpGetText function
from the httpsend unit
HTTP: THTTPSend;
begin
HTTP := THTTPSend.Create;
try
if HTTP.HTTPMethod('GET', 'https://www.paypal.com') then begin
caption := 'Success';
Memo1.Lines.LoadFromStream(HTTP.Document);
end else begin
caption := 'Failed';
writeln('LastError Code: ' + IntToStr(HTTP.Sock.LastError));
writeln('LastError Desc: ' + HTTP.Sock.LastErrorDesc);
end;
finally
HTTP.Free;
end;
end;
end.
[The writeln() method is just a utility routine thrown in, when I originally
thought I'd make a larger demo.]
Note first of all, that the SSL library is commented out, and we're
requesting an https: URL.
When you run the program, you will immediately notice a different problem,
one that's causing me some difficulties at the moment. The (F)LastError
integer property is being cleared on the way back out the call stack, so IT
CAN'T BE USED TO INSPECT the reasons for any failure. The string description
is still intact, but it would be much better to also have the code available
for error handling. In fact, when I study your code, it really looks like
you intended that. This is probably an oversight.
Let's trace through the important parts of what happens. To make it easier,
open the blcksock unit and place a breakpoint in the ReturnError method,
where TCustomSSL's FLastError is set to -1, and FLastErrorDesc is set to
"SSL is not implemented!" Running to that point, and then single-stepping
our way back out of the call stack we quickly arrive at SSLDoConnect where
the TTCPBlockSocket's FLastError is immediately "translated" into the wrong
(socket) error (10091) WSASYSNOTREADY. Going into ExceptCheck then changes
FLastErrorDesc to match the new FLastError ("Network subsystem is unusable"
which seems a decent "translation" but it's not intended for this). HOWEVER,
when the call stack unwinds further to bring us back into HTTPMethod(), the
failure to InternalConnect() makes the code close the socket... which would
be OK, EXCEPT that FLastError is CLEARED ( := 0) in several places [Purge()
and AbortSocket()].
I STRONGLY SUSPECT, but am not sure (especially about Purge), but I think
that the two "FLastError := 0" statements could safely be removed from both
Purge and AbortSocket. I think they NEED to be removed, in order for
LastError to have an accurate value at the end of these operations (like
LastErrorDesc).
Since Bind(), SetSin(), InternalCreateSocket(), SSLDoConnect() and other
"startup" functions DO initialize the FLastError values (:= 0), hopefully
removing those lines will enable true error checking based on the much more
usable code value.
------
ANOTHER interesting error-checking test can be done by removing the computer
from the network (pull the network cable or turn of WiFi access). Strangely
enough, you get a different error message (for exactly the same problem)
depending on which protocol is invoked:
https: Connection timed out (sometimes, depends on last access)
http: Host not found
When ideally, the response would be WSAHOSTUNREACH (10065) "No route to
host" or WSANETDOWN (10050) "Network is down."
Of course, the LastError is 0...
------
One way to handle the problem of reporting SSL errors, internal programming
configuration errors, and socket errors (among others) might be to add
another integer property that specifies the "class" (or "kind") of error
encountered. The FLastError value would then be interpreted in terms of this
ErrorKind value. [Example: when ErrorKind was ekSocketError, the
(F)LastError values would be the ones we see today.]
Alright, it's pretty late, I'm tired, and this message got way too long. I
hope it's understandable and helpful, and I'll provide whatever help I can
in resolving these issues.
Thank you.
P.S. I can't help mentioning another "language" issue that's been bothering
me for a long time: On the main Synapse web page there's a description line
for the "Current stable release" that says "Final release of Synapse." That
phraseology may well give casual visitors the impression that THAT'S the
LAST one (if they don't see that 38b7 is above...)! "Final" means the
absolute, ultimate end... which is obviously not the case...
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
synalist-public mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/synalist-public
