Sockets, WebClient, HttpClient and friends

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Sockets, WebClient, HttpClient and friends

timrowledge
Looking at the code I had a minor issue with the other day when loading from SqueakMap, I see what looks like a good opportunity to cleanup and simplify some classes. Since I'm most definitely not an expert in sockets and web interfaces etc I think some input from those of you more knowledgeable would be very helpful.

SqueakMap uses an HttpSocket directly to load; this has a problem in that the only error detection is to check the response for being a String. Which isn't the most robust or helpful technique. RBInternalSpellCheck does much the same, as do a few other users of #httpGet: (as an example).

InstallerInternetBased uses an http socket and checks the response for responding to #reset (I imagine as a proxy for stream-like behaviour?) which doesn't seem long-term robust.

Worst of all quite a few users of #httpGet: don't do any sort of checking and so will just dump users into a debugger; not our best look.

I know that WebClient is a somewhat heavier class but it does seem to do a lot more helpful things like handling redirects and so on as well as some better error testing. Would it be sensible to convert as much as practical to use WebClient instead of a plain HttpSocket? Might it be better to improve HttpSocket?

Some related stuff - there is an HttpClient class as well, unconnected to WebClient but used in several important looking places. Can that be improved or merged? At the very least it would be good to be able to document and comment code to explain why so many classes exist.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: TOAC: Turn Off Air Conditioner



Reply | Threaded
Open this post in threaded view
|

Re: Sockets, WebClient, HttpClient and friends

Tobias Pape
Hi Tim

> On 18.01.2019, at 02:29, tim Rowledge <[hidden email]> wrote:
>
> Looking at the code I had a minor issue with the other day when loading from SqueakMap, I see what looks like a good opportunity to cleanup and simplify some classes. Since I'm most definitely not an expert in sockets and web interfaces etc I think some input from those of you more knowledgeable would be very helpful.
>
> SqueakMap uses an HttpSocket directly to load; this has a problem in that the only error detection is to check the response for being a String. Which isn't the most robust or helpful technique. RBInternalSpellCheck does much the same, as do a few other users of #httpGet: (as an example).
>
> InstallerInternetBased uses an http socket and checks the response for responding to #reset (I imagine as a proxy for stream-like behaviour?) which doesn't seem long-term robust.
>
> Worst of all quite a few users of #httpGet: don't do any sort of checking and so will just dump users into a debugger; not our best look.
>
> I know that WebClient is a somewhat heavier class but it does seem to do a lot more helpful things like handling redirects and so on as well as some better error testing. Would it be sensible to convert as much as practical to use WebClient instead of a plain HttpSocket? Might it be better to improve HttpSocket?
>
> Some related stuff - there is an HttpClient class as well, unconnected to WebClient but used in several important looking places. Can that be improved or merged? At the very least it would be good to be able to document and comment code to explain why so many classes exist.
>


To puff the fog away a bit: Essentially everything uses WebClient.

SqueakMap uses WebClient: See SMClient>>client and SMClient class>>assureWebClient.
(there's a direct use of Socket and HTTPSocket in SMSqueakMap class>>pingServer: and #loadFullFrom: but onto that later)

InstallerInternetBased  also relies on HTTPSocket.

Same for any user of HTTPClient.


BUT: HTTPSocket now just "redirects" to WebClient. See: HTTPSocket class>>getDocument:args:accept:request:

"…"
"Some raw extra headers which historically have been added"
        xhdrs := HTTPProxyCredentials,
                HTTPBlabEmail, "may be empty"
                requestString. "extra user request. Authorization"

        client := WebClient new.
        ^[resp := client httpGet: urlString do:[:req|
                "Add ACCEPT header, accept plain text by default."
                req headerAt: 'Accept' put: (mimeType ifNil: ['text/html']).

                "Add the additional headers"
                (WebUtils readHeadersFrom: xhdrs readStream)
                        do:[:assoc| req addHeader: assoc key value: assoc value]].
"…"

TL;DR: It's just WebClient with differing layers of icing on top.

But, yes, it could all be a bit more clear cut.

Best regards
        -Tobias


> tim
> --
> tim Rowledge; [hidden email]; http://www.rowledge.org/tim
> Strange OpCodes: TOAC: Turn Off Air Conditioner
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: Sockets, WebClient, HttpClient and friends

timrowledge


> On 2019-01-18, at 1:30 AM, Tobias Pape <[hidden email]> wrote:
>
> TL;DR: It's just WebClient with differing layers of icing on top.

Hah; so it is. That's good, I guess. Evidently diving a dozen or so layers down wasn't enough to discover that. ;-)

>
> But, yes, it could all be a bit more clear cut.

It certainly could; but more importantly it could do much better for raising and handling errors helpfully. I note with wry amusement how the last part of HTTPSocket class>>#httpGetDocument:args:accept:request: carefully takes the helpful error info of WebClient and ensures that the less helpful HttpSocket response is generated.

And of course none of this alters the fact that a good few methods could do with changing to handle network errors, whichever flavour of WebClient icing they use!


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Useful random insult:- Proof that evolution CAN go in reverse.