Web crawler in F#

Question

I have been writing a web crawler in F# that downloads pages with stylesheets and scripts.
Can somebody give me suggestions on improving this code, please? Would appreciate any feedback that could improve it / prettify it.

open System
open System.Net

let log fmt =
    Printf.kprintf (fun str ->   // todo: lock | agent
        printfn "%-4O %-10O %O" Environment.CurrentManagedThreadId (DateTime.Now.ToLongTimeString()) str
        ) 
        fmt

type File = {    
    ContentType : string; 
    Path : string 
    }                   

type DownloadResult = 
| Error of exn
| Content of File

module files = 
    let download trgFileName srcUrl = async {    
        try
            let w = WebRequest.Create(Uri srcUrl) 
            use! r = w.AsyncGetResponse()
            use f = new IO.FileStream(trgFileName, IO.FileMode.Create, IO.FileAccess.Write, IO.FileShare.None)
            r.GetResponseStream().CopyTo f
            return Content { ContentType = r.ContentType; Path = trgFileName }
        with e -> 
            return Error e
        }

module filesTest = 
    let download trgFileName srcUrl = async {     
        return Error (exn "todo")
        }

let computeHash (s:IO.Stream) = 
    Security.Cryptography.HashAlgorithm.Create().ComputeHash s

let fileHash (path:string) = 
    use fs = new IO.FileStream(path, IO.FileMode.Open, IO.FileAccess.Read)
    let hc = computeHash fs
    BitConverter.ToString(hc).Replace("-", "")

let trimQuery url = 
    Uri(url).GetLeftPart(UriPartial.Path)

open files   
// open filesTest    

let urls htmlPath : string list = [
    // todo: extract links from html. Load HtmlDocument and travers DOM nodes
    ]

let crawl trgFolder maxDepth url = 
    let cs = new Threading.CancellationTokenSource()
    let m = MailboxProcessor<int * string>.Start(fun inbox -> async {      // depth * url
        use entries = new IO.StreamWriter(IO.Path.Combine(trgFolder, "entries.txt"), true)
        let visited = Collections.Generic.HashSet<string>()
        while true do
            let! depth, url = inbox.Receive()
            if depth < maxDepth then  
                let url' = trimQuery url
                if not(visited.Add url') then
                    log "downloading %O" url
                    let! r = download (IO.Path.GetTempFileName()) url'
                    match r with 
                    | Error e -> log "error %O" e.Message
                    | Content f ->  // todo: async { } |> Async.Start  + cs.Token  + visited - Concurrent.ConcurrentDictionary
                        let hc = fileHash f.Path
                        log "processing %O as %O" url hc
                        let path = IO.Path.Combine(trgFolder, hc)
                        IO.File.Move(f.Path, path)
                        entries.WriteLine(sprintf "%O \t %O" hc url)
                        for u in urls path |> Seq.map trimQuery do
                            if visited.Add u then 
                                inbox.Post (depth+1, u)
            if inbox.CurrentQueueLength = 0 then 
                log "done"
                cs.Cancel()
        }, cs.Token)
    m.Post (0, url)
    cs

let c = crawl __SOURCE_DIRECTORY__ 2 "https://gist.github.com" 
// c.Cancel()

Answer 1

The first, purely aesthetic impression that strikes me is the level of nesting in your main crawl function. Consider extracting the core of that function. This might only be to a local function within crawl, but trying to make functions which have less than 7 (*or some fairly small number) generally makes code easier to follow.

Consider wrapping primitives in single-case discriminated unions to make passing variables safer. E.g. you could create a type to wrap the string url which is passed around such as:

type Url = Url of string

Another option to reduce nesting is to pass a function into crawl which will be executed having completed the request, so would be of signature DownloadResult -> Url list (or maybe DownloadResult -> Async<Url list>) allowing you to return the set of next URLs to fetch. By doing this you've decoupled the ability to fetch a URL from what you ultimately want to do with the result. This pattern is the functional equivalent of the dependency inversion principle.

I'd question why you've chosen to write all content straight to disk then pass the file path around. Perhaps you know that you're planning on downloading large files, but I'd suggest for a first implementation it might be simpler to just pass an array of bytes or a stream around. My second thought would be to pass the response stream in the DownloadResult. This allows you to delegate to the handler what it's going to do with the content, rather that build that assumption into crawl.

Answer 2

Actually web crawling is a quite large topic. First, about your existing code.

1. Mostly cosmetic edits. I would recommend payload functionality (moving content to files) to separate function and pass it as parameter.

2. MailboxProcessor is a good decision for a crawler, but it seems you did not implement it properly if I did not miss something - the main issue is that agent will be busy until it will have not completed current page processing. To solve it I would execute the following piece of code in Async.StartChild.

   let! r = download (IO.Path.GetTempFileName()) url'
                       match r with 
                       | Error e -> log "error %O" e.Message
                       | Content f ->  // todo: async { } |> Async.Start  + cs.Token  + visited - Concurrent.ConcurrentDictionary
                           let hc = fileHash f.Path
                           log "processing %O as %O" url hc
                           let path = IO.Path.Combine(trgFolder, hc)
                           IO.File.Move(f.Path, path)
                           entries.WriteLine(sprintf "%O \t %O" hc url)
                           for u in urls path |> Seq.map trimQuery do
                               if visited.Add u then 
                                   inbox.Post (depth+1, u)
   

   Second is about improvements specific for crawling.

3. I do not see the delay between requests - many sites will ban you for that.

4. You did not post code for links extraction and handling. It may be tricky to implement it properly.

5. I would set user agent to prevent blocking by some servers (though, it is not such important as delay).

Also, I recently wrote two blog posts about the topic (one about theoretical aspects and second about implementation in F#) - maybe you will find something useful, especially in parts of delays implementation, URL links extraction and proper asynchronous handling.