att / rcloud

Collaborative data analysis and visualization
http://rcloud.social
MIT License
430 stars 142 forks source link

review `proxy.R` #2665

Open s-u opened 5 years ago

s-u commented 5 years ago

proxy.R was a quick hack / proof of concept which is now being used increasingly. A couple issues to review:

  1. it uses direct call to session.init.rcs() so it can get session info (to determine the host). This is not guaranteed to work.
  2. when passing form POST requests they get parsed and need to be de-parsed which is potentially error-prone
  3. 2586 added support for passing through headers other than Content-length, but it needs some cleaning, that option is undocumented and the comments in proxy.R are now out of date.

  4. there is no logging of requests
gordonwoodhull commented 5 years ago

We also preserve the "passthrough" headers, which are configurable - see #2586.

s-u commented 5 years ago

ok, thanks, I've added it to the list

gordonwoodhull commented 4 years ago

It also seems that if there is an error in proxy.R, it returns the error as content with response code 200.

s-u commented 4 years ago

partially addressed in #2693 (logging of requests and errors 0 note that requests have been always logged as call.script entries) for 2.2, moving the rest to later