Is it safe to assume InternetCloseHandle() won't fail, thereby allowing cleaner code?

Here's a routine to do an HTTP request using WinINet and either return the fetched string or raise an exception:

function Request(const pConnection: HINTERNET; const localpath: string): string;
var Buffer: packed Array[1..5000] of Char; BytesRead: Cardinal; pRequest: HINTERNET;     sent: boolean;
begin
Result := '';
pRequest := HTTPOpenRequest(pConnection, 'GET', pchar(localpath), nil, nil, nil, 0, 0);
if pRequest <> nil then
  begin
  sent := HTTPSendRequest(pRequest, nil, 0, nil, 0);
  开发者_开发知识库if sent then
    while InternetReadFile(pRequest, @Buffer, SizeOf(Buffer)-1 {leave room for terminator}, BytesRead) do
      begin
      Buffer[BytesRead+1] := #0;
      Result := Result + buffer;
      end;
  InternetCloseHandle(pRequest);
  if not sent then RaiseLastOSerror; // HTTPSendRequest failed
  end
else RaiseLastOSerror; // HTTPOpenRequest failed
end;

If InternetCloseHandle(pRequest) can fail even though pRequest was successfully assigned, GetLastError() will return an error code for InternetCloseHandle() instead of HTTPSendRequest(). Fixing that would require code like:

function Request(const pConnection: HINTERNET; const localpath: string): string;
var Buffer: packed Array[1..5000] of Char; BytesRead: Cardinal; pRequest: HINTERNET;
begin
Result := '';
pRequest := HTTPOpenRequest(pConnection, 'GET', pchar(localpath), nil, nil, nil, 0, 0);
if pRequest <> nil then
  begin
  if HTTPSendRequest(pRequest, nil, 0, nil, 0) then
    while InternetReadFile(pRequest, @Buffer, SizeOf(Buffer)-1 {leave room for terminator}, BytesRead) do
      begin
      Buffer[BytesRead+1] := #0;
      Result := Result + buffer;
      end
  else
    begin
    InternetCloseHandle(pRequest);
    RaiseLastOSerror; // HTTPSendRequest failed
    end;
  InternetCloseHandle(pRequest);
  end
else RaiseLastOSerror; // HTTPOpenRequest failed
end;

but that seems a lot uglier and more confusing at first glance.

Is it safe to assume InternetCloseHandle() won't fail, thereby allowing the simpler code?

First of all this kind of code is not helpful:

raise Exception.Create(IntToStr(GetLastError))

Use this instead:

RaiseLastOsError; // This raises an exception with the description of the error

Since you're using exceptions in your code, how about calling the function like this so it raises an exception if the handle can't be closed?

Win32Check(InternetCloseHandle(H))

I think you are going about this the wrong way. You should simply check for errors on each API call and raise an exception as soon as you encounter one. That way you get the error message appropriate to the error that produces the exception. You simply can't expect to carry on calling other API functions and then raise an exception for an error that happened some time ago.

I think you want something along these lines:

Result := '';
pRequest := HTTPOpenRequest(pConnection, 'GET', pchar(localpath), nil, nil, nil, 0, 0);
if pRequest=nil then
  RaiseLastOSerror;
try
  CheckWin32Error(HTTPSendRequest(pRequest, nil, 0, nil, 0));
  while InternetReadFile(pRequest, @Buffer, SizeOf(Buffer)-1, BytesRead) do begin
    Buffer[BytesRead+1] := #0;
    Result := Result + buffer;
  end;
  if GetLastError<>0 then
    RaiseLastOSerror;
finally
  CheckWin32Error(InternetCloseHandle(pRequest));
end;

Note that you didn't include any error checking for InternetReadFile. I've attempted to write it for you.

You need to make sure that a handle is 'closed' if it is successfully created. That is, you should do

hReq := HTTPOpenRequest(...);
if hReq <> 0 then
  try      
    // Do stuff
  finally
    InternetCloseHandle(hReq);
  end;

Looking at SOAPHTTPTrans, there are many calls to InternetCloseHandle. Although many appear within Except blocks, none are PROTECTED by one. So it appears that Codegear (mine is D2006) assumes that it won't fail.