Skip to content

ETagManager: add API key to ETag content#2665

Closed
NachoSoto wants to merge 1 commit into
mainfrom
etag-manager-api-key
Closed

ETagManager: add API key to ETag content#2665
NachoSoto wants to merge 1 commit into
mainfrom
etag-manager-api-key

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

This fixes the following scenario:

  • User makes initial requests with invalid API key
  • 401 response
  • We store that response (ErrorResponse)
  • User fixes the API key
  • Server returns 304
  • We return the cached body for the ErrorResponse

This is a proposed fix for COIN-595. Instead of this, we might want to ensure that 401 responses don't return etags.

This fixes the following scenario:

- User makes initial requests with invalid API key
- 401 response
- We store that response (`ErrorResponse`)
- User fixes the API key
- Server returns 304
- We return the cached body for the `ErrorResponse`
@NachoSoto NachoSoto added the pr:fix A bug fix label Jun 15, 2023
@NachoSoto NachoSoto requested review from a team and bisho June 15, 2023 23:23
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Closing this since the backend is no longer sending etags in 401s.

@NachoSoto NachoSoto closed this Jun 16, 2023
@NachoSoto NachoSoto deleted the etag-manager-api-key branch June 16, 2023 16:25
NachoSoto added a commit that referenced this pull request Jun 21, 2023
We had lots of different parts of the code doing `request.url?.absoluteString` and also using different values for e-tags.
This refactor combines all that to use a single `eTag(for:)` method.

I did this as part of #2665, which is getting closed, but it will still be useful if we ever decide to change the content of e-tags.
NachoSoto added a commit that referenced this pull request Jun 21, 2023
We had lots of different parts of the code doing
`request.url?.absoluteString` and also using different values for
e-tags.
This refactor combines all that to use a single `eTag(for:)` method.

I did this as part of #2665, which is getting closed, but it will still
be useful if we ever decide to change the content of e-tags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant