Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip caching failed dumps of http responses #380

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrejgou
Copy link

Errors when dumping the response are ignored so an empty value is cached, which causes deserialization of the response to fail with an error, which is also ignored, which causes a nil pointer panic.

Summary

Okta HTTP requests were occasionally failing with nil pointer panics. The stack trace showed the panic originating from this line:

ct := resp.Header.Get("Content-Type")

called from this line:
return buildResponse(resp, re, &v)

The only way the HTTP response can be nil is if deserialization fails, which can only fail if the cached value is invalid, which can only happen if serialization of the HTTP response fails when setting the value in the cache:
cacheableResponse, _ := httputil.DumpResponse(value, true)

The easiest fix I could think of was to just skip setting the cache for values that failed serialization. The only potential impact is additional cache misses, but only for invalid values that aren't usable anyway.

Fixes N/A

Type of PR

  • Bug Fix (non-breaking fixes to existing functionality)
  • New Feature (non-breaking changes that add new functionality)
  • Documentation update
  • Test Updates
  • Other (Please describe the type)

Test Information

  • My PR required test updates

Go Version:
Os Version:
OpenAPI Spec Version:

Signoff

  • I have submitted a CLA for this PR
  • Each commit message explains what the commit does
  • I have updated documentation to explain what my PR does
  • My code is covered by tests if required
  • I ran make fmt on my code
  • I did not edit any automatically generated files

Errors when dumping the response are ignored so an empty value
is cached, which causes deserialization of the response to fail with
an error, which is also ignored, which causes a nil pointer panic.
@github-actions
Copy link

This PR has been marked stale because there has been no activity within the last 28 days. To keep this PR active, remove the stale label.

@github-actions github-actions bot added the stale label Jun 22, 2023
@monde monde removed the stale label Jul 4, 2023
@github-actions
Copy link

github-actions bot commented Aug 1, 2023

This PR has been marked stale because there has been no activity within the last 28 days. To keep this PR active, remove the stale label.

@github-actions github-actions bot added the stale label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants