Browse Source

Minor Proxy Improvements (#1289)

* fix(serve): provide a clear error with pathless proxy URLs

Without this, `dx serve` panics with this message:

```
Paths must start with a `/`. Use "/" for
 root routes
 ```

 That's not very clear. Instead, we can detect this situation and provide a better error message:

```
Error: 🚫 Serving project failed: Failed to establish proxy: Proxy backend URL must have a non-empty path, e.g. http://localhost:8080/api instead of http://localhost:8080
```

* docs(config): correct format for `web.proxy`
Brian Donovan 1 year ago
parent
commit
6d154b5072

+ 1 - 1
packages/cli/docs/src/configure.md

@@ -144,7 +144,7 @@ Configeration related to static resources your application uses in development:
 Configeration related to any proxies your application requires durring development. Proxies will forward requests to a new service
 Configeration related to any proxies your application requires durring development. Proxies will forward requests to a new service
 
 
 ```
 ```
-[web.proxy]
+[[web.proxy]]
 # configuration
 # configuration
 ```
 ```
 
 

+ 3 - 0
packages/cli/src/error.rs

@@ -35,6 +35,9 @@ pub enum Error {
     #[error("Invalid proxy URL: {0}")]
     #[error("Invalid proxy URL: {0}")]
     InvalidProxy(#[from] hyper::http::uri::InvalidUri),
     InvalidProxy(#[from] hyper::http::uri::InvalidUri),
 
 
+    #[error("Failed to establish proxy: {0}")]
+    ProxySetupError(String),
+
     #[error("Error proxying request: {0}")]
     #[error("Error proxying request: {0}")]
     ProxyRequestError(hyper::Error),
     ProxyRequestError(hyper::Error),
 
 

+ 29 - 2
packages/cli/src/server/web/proxy.rs

@@ -48,6 +48,16 @@ impl ProxyClient {
 pub fn add_proxy(mut router: Router, proxy: &WebProxyConfig) -> Result<Router> {
 pub fn add_proxy(mut router: Router, proxy: &WebProxyConfig) -> Result<Router> {
     let url: Uri = proxy.backend.parse()?;
     let url: Uri = proxy.backend.parse()?;
     let path = url.path().to_string();
     let path = url.path().to_string();
+    let trimmed_path = path.trim_end_matches('/');
+
+    if trimmed_path.is_empty() {
+        return Err(crate::Error::ProxySetupError(format!(
+            "Proxy backend URL must have a non-empty path, e.g. {}/api instead of {}",
+            proxy.backend.trim_end_matches('/'),
+            proxy.backend
+        )));
+    }
+
     let client = ProxyClient::new(url);
     let client = ProxyClient::new(url);
 
 
     // We also match everything after the path using a wildcard matcher.
     // We also match everything after the path using a wildcard matcher.
@@ -56,7 +66,7 @@ pub fn add_proxy(mut router: Router, proxy: &WebProxyConfig) -> Result<Router> {
     router = router.route(
     router = router.route(
         // Always remove trailing /'s so that the exact route
         // Always remove trailing /'s so that the exact route
         // matches.
         // matches.
-        path.trim_end_matches('/'),
+        trimmed_path,
         any(move |req| async move {
         any(move |req| async move {
             client
             client
                 .send(req)
                 .send(req)
@@ -68,7 +78,7 @@ pub fn add_proxy(mut router: Router, proxy: &WebProxyConfig) -> Result<Router> {
     // Wildcard match anything else _after_ the backend URL's path.
     // Wildcard match anything else _after_ the backend URL's path.
     // Note that we know `path` ends with a trailing `/` in this branch,
     // Note that we know `path` ends with a trailing `/` in this branch,
     // so `wildcard` will look like `http://localhost/api/*proxywildcard`.
     // so `wildcard` will look like `http://localhost/api/*proxywildcard`.
-    let wildcard = format!("{}/*proxywildcard", path.trim_end_matches('/'));
+    let wildcard = format!("{}/*proxywildcard", trimmed_path);
     router = router.route(
     router = router.route(
         &wildcard,
         &wildcard,
         any(move |req| async move {
         any(move |req| async move {
@@ -168,4 +178,21 @@ mod test {
     async fn add_proxy_trailing_slash() {
     async fn add_proxy_trailing_slash() {
         test_proxy_requests("/api/".to_string()).await;
         test_proxy_requests("/api/".to_string()).await;
     }
     }
+
+    #[test]
+    fn add_proxy_empty_path() {
+        let config = WebProxyConfig {
+            backend: "http://localhost:8000".to_string(),
+        };
+        let router = super::add_proxy(Router::new(), &config);
+        match router.unwrap_err() {
+            crate::Error::ProxySetupError(e) => {
+                assert_eq!(
+                    e,
+                    "Proxy backend URL must have a non-empty path, e.g. http://localhost:8000/api instead of http://localhost:8000"
+                );
+            }
+            e => panic!("Unexpected error type: {}", e),
+        }
+    }
 }
 }