throw timeout error not as XHRError Exn.Error, but as separate error

[srghma]

Description of the change

fixes #155

---

Checklist:

• Added the change to the changelog's "Unreleased" section with a link to this PR and your username
• Linked any existing issues or proposals that this pull request should close
• Updated or added relevant documentation in the README and/or documentation directory
• Added a test for the contribution (if applicable)

[thomashoneyman]

I'm on board with this, though it would be good to get another look from @garyb or @eric-corumdigital if y'all have a moment.

[thomashoneyman]

Just one request wrt minimizing the diff, and otherwise 👍 I'm happy to merge this.

[thomashoneyman]

Could you return this function to its prior formatting, ie.

request :: forall a. Request a -> Aff (Either Error (Response a))
request req = 
  case req.content of
    Nothing ->
      send (toNullable Nothing)
    Just content ->
      case extractContent content of
        Right c ->
          send (toNullable (Just c))
        Left err ->
          pure $ Left (RequestContentError err)

It's not necessary to move ajaxRequest, send, headers, and fromResponse out of the where clause where they were before (I tested moving them back locally and things compile just fine). Can you move them back? That will minimize the diff and prevent us from splitting some functions into let bindings and others underneath where.

[srghma]

Ok, I write why I did this - for speed and makes easier to see what is dependent on args and what is not

Fix anyway?

[srghma]

@thomashoneyman

[thomashoneyman]

I haven't examined the output code -- in what way does this improve performance? I'm genuinely curious.

[srghma]

currently it compiles to

https://pastebin.com/PFU8femC

now

var request = (function () {
    var notDependentOnArg1 = function (v) { ... }
    var notDependentOnArg2 = function (v) { ... }
    return function(req) {
      var dependentOnArg1 = function (v) { ... req ... }
      var dependentOnArg2 = function (v) { ... req ... }

      ...
    }
})()

request = \req ->
  let
    dependentOnArg1 = ..
    dependentOnArg2 = ..
  in ...
  where
    notDependentOnArg1 = ..
    notDependentOnArg2 = ..

before

var request = function(req) {
  var notDependentOnArg1 = function (v) { ... }
  var notDependentOnArg2 = function (v) { ... }

  var dependentOnArg1 = function (v) { ... req ... }
  var dependentOnArg2 = function (v) { ... req ... }

  ...
}

request req = ...
  where
    dependentOnArg1 = ..
    dependentOnArg2 = ..

    notDependentOnArg1 = ..
    notDependentOnArg2 = ..

[kritzcreek]

Premature optimization is the root of all evil (or at least most of it) in programming

This is code that is always run in conjunction with a network request. You'll never be able to notice any difference here because network latency dominates a couple of closure allocations, so readability/uniformity has way higher priority.

I can appreciate the argument about making the scoping clearer, but I don't think mixing let and where like that actually does that, because you have to read from the top and bottom to find the sandwiched piece of logic in the middle.

[srghma]

example of XHROtherError

Error__AffjaxError {
  value0: XHROtherError {
    value0: TypeError [ERR_UNESCAPED_CHARACTERS]: Request path contains unescaped characters
        at new ClientRequest (_http_client.js:151:13)
        at Object.request (https.js:313:10)
        at XMLHttpRequest._sendHxxpRequest (/home/srghma/projects/extract-pdf-notes/node_modules/xhr2/lib/xhr2.js:480:24)
        at XMLHttpRequest._sendHttp (/home/srghma/projects/extract-pdf-notes/node_modules/xhr2/lib/xhr2.js:460:14)
        at XMLHttpRequest.send (/home/srghma/projects/extract-pdf-notes/node_modules/xhr2/lib/xhr2.js:278:18)
        at /home/srghma/projects/extract-pdf-notes/output/Affjax/foreign.js:84:11
        at __do (/home/srghma/projects/extract-pdf-notes/output/Effect.Aff.Compat/index.js:15:22)
        at runAsync (/home/srghma/projects/extract-pdf-notes/output/Effect.Aff/foreign.js:98:20)
        at run (/home/srghma/projects/extract-pdf-notes/output/Effect.Aff/foreign.js:333:22)
        at /home/srghma/projects/extract-pdf-notes/output/Effect.Aff/foreign.js:348:19 {
      code: 'ERR_UNESCAPED_CHARACTERS'
    }
  }
}

if one forgets to use JSURI.encodeURIComponent