[v3] Errors getting swallowed by JWK cache about jwx HOT 3 CLOSED

I think my original intention was that you would at least call c.Refresh() -- possibly multiple times -- before using c.Get().
That basically means that you would at least warm the cache once in order to avoid this situation.

IIRC the main reason for this was that I thought it was hard for users to distinguish initial fetch errors/refreshing errors/genuine parse errors if you used it directly in a loop without warming it first

//pseudocode

// something _like_ this,
for {
   if !c.IsRegistered(url) {
      _ := c.Register(url)
      _, err := c.Refresh(url)
      if err != nil { c.Unregister(url) }
      // error case, halt or ignore this iteration
      continue
  } else {
     c.Get(url)
  }
}

// vs

for {
  _, err := c.Get(url)
}

Now, I'm fairly certain there are practical usecases that I missed, so please feel free to convince me otherwise if the above does not fit your needs.

from jwx.

Hey thanks for the response @lestrrat! And thanks again for maintaining the project..In our case, its possible to hit the same 403 (or related) error even when warming the cache, in which case the 403 is swallowed and the second parse error appears the first time we do a get.

Thankfully the functionality exists in the library to pass an http client, so we ended up just creating a simple client implementation that checks status codes and returns relevant errors.

Thanks again

from jwx.

@JulianRushdown I'm glad you were able to work your problem out, however I was a bit concerned with the fact that you were saying even warming the cache "swallows" the error, so I checked with a test:

func TestGH922(t *testing.T) {
	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusForbidden)
	}))
	defer srv.Close()

	// Refresh should fail, but we should receive the error
	c := jwk.NewCache(ctx)
	c.Register(srv.URL)
	
	// Run these checks multiple times so that we can be sure that the cache is
	// NOT being used
	for i := 0; i < 5; i++ {
		// Refresh should fail, but we should receive the 403 error
		_, err := c.Refresh(ctx, srv.URL)
		require.Error(t, err, `c.Refresh should fail`)
		// t.Logf("%s", err)
	}
}

In the above test c.Refresh does return an error, so you should be able to warm the cache while catching errors.

Now, if I print out the error by uncommenting the t.Logf line, I get something like jwk_test.go:2152: failed to fetch "http://127.0.0.1:35339": failed to transform HTTP response for "http://127.0.0.1:35339": failed to parse JWK set at "http://127.0.0.1:35339": failed to unmarshal JWK set: EOF -- which now finally clicked in me: by being "swallowed", do you mean to say that the error message is not indicative of the non-200 HTTP response? I always interpreted your report as "there's no way to obtain the error object"

If the error message is the problem I can certainly change the above error to look like

jwk_test.go:2152: failed to fetch "http://127.0.0.1:42521": failed to transform HTTP response for "http://127.0.0.1:42521": failed to process response: non-200 response code "403 Forbidden"

Is this (at least part of) what you would like?

from jwx.