995 posts
Location
Canada
Posted 27 April 2014 - 02:01 AM
Since the very limited default whitelist made it into a CC release, I'm gonna put this in suggestions.
Currently, the HTTP API errors the program when the URL accessed isn't on the whitelist. If Dan doesn't want to queue an "http_failure" instead of an error message, could we at least have more helpful error messages? Something like "The url (url accessed) isn't on the whitelist. Add it to the whitelist in ComputerCraft.cfg."
I don't see why this can't be implemented, it doesn't modify the current http whitelist or it's behaviour, just returns something more helpful. Maybe we could see better messages for other things, too, but this one really won't make much sense to a new user.
2151 posts
Location
Auckland, New Zealand
Posted 27 April 2014 - 03:10 AM
Also, I know it's not completely related, but Dan please add GitHub to the default whitelist!
1140 posts
Location
Kaunas, Lithuania
Posted 27 April 2014 - 07:05 AM
Error the program when you can't access a site is annoying. Wasn't 'http_failure' event created just for that? It was meant to be thrown when there is something wrong and you can't connect to a site.
7508 posts
Location
Australia
Posted 27 April 2014 - 08:11 AM
I definitely think http_failure event definitely would be the ideal solution here…
7083 posts
Location
Tasmania (AU)
Posted 27 April 2014 - 08:52 AM
I dunno, apparently that event (and the corresponding success event) are supposed to be captured by the "get" command itself… both would need to be "revealed" in order to make it viable.
And catching events for this sort of thing feels messy to me anyway, at least, when compared to the option of using pcall…
If it were to be changed, I'd think it more suitable to just have it return a boolean set to false along with a string containing the detailed error message (which is how at least some other ComputerCraft functions - notably those from the turtle API - already report failure).
7508 posts
Location
Australia
Posted 27 April 2014 - 09:24 AM
-snip-
Well what you're discussing here is what already happens, turning the whitelist error into a
http_failure would be better, take a look at the
wrapRequest function used by
http.get and
http.post
local function wrapRequest( _url, _post, _headers )
http.request( _url, _post, _headers )
while true do
local event, param1, param2 = os.pullEvent()
if event == "http_success" and param1 == _url then
return param2
elseif event == "http_failure" and param1 == _url then
return nil
end
end
end
could easily have
http_failure return a string as to why it failed, then return it with
nil.
Edited on 27 April 2014 - 07:25 AM
1140 posts
Location
Kaunas, Lithuania
Posted 27 April 2014 - 09:50 AM
Actually, if dan200 for some reason doesn't want to add an error thru an event another sollution would be pcall'ing http.request inside wrapRequest function and return false/nil + error message. This way any error messages returned by http.request could be safely handled, even those which may appear in the future.
7083 posts
Location
Tasmania (AU)
Posted 27 April 2014 - 10:33 AM
Well what you're discussing here is what already happens
Gotcha. I was getting the impression coders here wanted to handle the event themselves.
7508 posts
Location
Australia
Posted 27 April 2014 - 10:51 AM
Gotcha. I was getting the impression coders here wanted to handle the event themselves.
Well obviously in
http.request you'd have to handle the event yourself, but using
http.get and
http.post they have the wrapper.
1522 posts
Location
The Netherlands
Posted 28 April 2014 - 01:00 PM
I'd like to expand to this, and add a native function: isOnWhitelist or something in that manner.
Because most programs will think it can't reach the net if it throws a http_failure event, so they can input their website into the function and notify the user that (s)he should add it to the configuration to make the program work.
Otherwise there won't be a way of properly notifying the user about the website, other than putting it in the topic. Not everyone reads every topic on a program and just blindly tests it out
995 posts
Location
Canada
Posted 29 April 2014 - 01:25 AM
I think that a optional blacklist would work better than a whitelist though, maybe an option to switch between the two.
171 posts
Location
Eastern USA
Posted 30 April 2014 - 10:35 PM
Instead of an isWhitelisted function, how about making the http_failure event return as a second parameter a reason for failure, like http disabled, no DNS server, your computer has no IP address, no route to host, host not whitelisted, etc.
Also, can you make http requests return http_disabled as a second parameter if http is disabled instead of the http table just not existing? That way, a program can just make an http request, and when it fails, give up (or, if it retries on failure, look at the reason and decide it's useless to try again). It won't have to do 2 checks - one if http exists and another if it succeeded or failed. It will also be better for newbies, because then there will be one less opportunity for attempt to index nil errors (because http will never be nil).
Edited on 30 April 2014 - 08:36 PM
8543 posts
Posted 30 April 2014 - 10:49 PM
The http table not existing when disabled is fine, since handling that case can be built in without a second if, unless you want to specifically tell the user that http is disabled, in which case you'd need a second if anyway, regardless of which way it's implemented. It is more consistent to simply not have the API for disabled features be present at all.
local remoteHandle = http and http.get("http://www.example.com/")