Update for form-urlencoded v5

purescript-contrib / purescript-affjax

An asynchronous AJAX library built using Aff.

Apache License 2.0

122 stars 79 forks source link

Update for form-urlencoded v5 #141

Closed garyb closed 5 years ago

garyb commented 5 years ago

Related: #139

The specific issue there is fixed, but by making the current affjax error at compile time with the new form-urlencoded.

nsaunders commented 5 years ago

From what I gather, there's a bit of a design challenge here in that the request signature doesn't currently have a way to surface an error that might occur when constructing an AjaxRequest. Are there any ideas about what such an API should look like?

garyb commented 5 years ago

I think it's ok - the problem would occur before request, since it's when constructing the value for content that this failure would occur - the appropriate constructor for RequestBody expects a FormURLEncoded typed value, it doesn't take a string and encode that or anything.

I'll try doing the update now, I meant to do it ages ago but have had various things going on. Will find out quickly enough if it is a problem or not!

garyb commented 5 years ago

Ok, I'm not very good at reading comprehension obviously, since there's a line right there that says FormURLEncoded.encode 😄. I'll figure something out though, only just started looking.

garyb commented 5 years ago

I think I'm going to change the design a bit in a way that will help with a footgun affjax has right now: when an error occurs making the XHR request, it explodes using the Aff error channel (like in #128). So I'm going to make a Result ADT for request than can encompass a couple of different possibilities:

Request content error (for this new case)
XHR error (for catching throws when making the XHR request - bad URLs, things like that)
Response format error (this is captured as a Response (Either ...) right now - it will still have the response attached, but with a Foreign body since decoding it failed)
Success

It means there'll be a bit more ceremony to handling results, but at least it will cover all the possible failure states now, and won't have to fall back on silently disappearing request content that is bad or something.

nsaunders commented 5 years ago

Thanks for looking into this @garyb. I'm sorry that the changes I made to form-urlencoded created more work for you, but I do think that the new interface you have described is a definite improvement considering the things that can go wrong.

garyb commented 5 years ago

Resolved by #145 🙂

nsaunders commented 5 years ago

Awesome, thanks @garyb!