Repository: megaease/easegress
Pull request #537 (2022-03-10 07:16:01)

fix(proxy): fallback not working
https://github.com/megaease/easegress/blob/36480f34946b464820360ea1a535880fd7e7751f/pkg/filter/proxy/proxy.go#L351-L360

As shown above, handle processing results equal to `""` on behalf of the request is normal, `not ""` on behalf of the occurrence of errors, fallback mechanism is said to occur after the error processing mechanism, so it should be `result ! = ""` in the case of fallback processing

codecov-commenter: # [Codecov](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease) Report
> Merging [#537](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease) (3c18fd9) into [main](https://codecov.io/gh/megaease/easegress/commit/8649d0043063212e860e5c30c999917e6cc16811?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease) (8649d00) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 3c18fd9 differs from pull request most recent head 3f4f7a3. Consider uploading reports for the commit 3f4f7a3 to get more accurate results

[![Impacted file tree graph](https://codecov.io/gh/megaease/easegress/pull/537/graphs/tree.svg?width=650&height=150&src=pr&token=5Q80B98LPI&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease)](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease)

```diff
@@            Coverage Diff             @@
##             main     #537      +/-   ##
==========================================
+ Coverage   80.56%   80.59%   +0.02%     
==========================================
  Files          88       88              
  Lines       10123    10123              
==========================================
+ Hits         8156     8159       +3     
+ Misses       1514     1512       -2     
+ Partials      453      452       -1     
```


| [Impacted Files](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease) | Coverage Δ | |
|---|---|---|
| [pkg/filter/proxy/proxy.go](https://codecov.io/gh/megaease/easegress/pull/537/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease#diff-cGtnL2ZpbHRlci9wcm94eS9wcm94eS5nbw==) | `80.20% <100.00%> (+1.56%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease). Last update [36480f3...3f4f7a3](https://codecov.io/gh/megaease/easegress/pull/537?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=megaease).


Patch:
diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go
index e8bb0cc6..37cbfea4 100644
--- a/pkg/filter/headerlookup/headerlookup.go
+++ b/pkg/filter/headerlookup/headerlookup.go
@@ -162,7 +162,7 @@ func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) {
 		return nil, err
 	}
 	if etcdVal == nil {
-		return nil, fmt.Errorf("no data found")
+		return nil, fmt.Errorf("no data for key %s found", hl.etcdPrefix+headerVal)
 	}
 	result := make(map[string]string, len(hl.spec.HeaderSetters))
 	etcdValues := make(map[string]string)
diff --git a/pkg/filter/proxy/proxy.go b/pkg/filter/proxy/proxy.go
index ebe525b2..5278f2ad 100644
--- a/pkg/filter/proxy/proxy.go
+++ b/pkg/filter/proxy/proxy.go
@@ -350,13 +350,12 @@ func (b *Proxy) handle(ctx context.HTTPContext) (result string) {
 
 	result = p.handle(ctx, ctx.Request().Body(), b.client)
 	if result != "" {
+		if b.fallbackForCodes(ctx) {
+			return resultFallback
+		}
 		return result
 	}
 
-	if b.fallbackForCodes(ctx) {
-		return resultFallback
-	}
-
 	// compression and memoryCache only work for
 	// normal traffic from real proxy servers.
 	if b.compression != nil {


Test patch:
diff --git a/pkg/filter/proxy/proxy_test.go b/pkg/filter/proxy/proxy_test.go
index a2132163..429d0d86 100644
--- a/pkg/filter/proxy/proxy_test.go
+++ b/pkg/filter/proxy/proxy_test.go
@@ -143,6 +143,16 @@ failureCodes: [503, 504]
 		t.Error("proxy.Handle should fail")
 	}
 
+	// test fallback
+	ctx.MockedResponse.MockedStatusCode = func() int {
+		return http.StatusServiceUnavailable
+	}
+
+	result = proxy.Handle(ctx)
+	if result != "fallback" {
+		t.Error("proxy.Handle should fallback")
+	}
+
 	time.Sleep(10 * time.Millisecond)
 	proxy.Close()
 	time.Sleep(10 * time.Millisecond)