Add skipPaths to selectively skip requests from Chucker
#970
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'd like to test this out @ArjanSM on the sample app but I've just noticed it instacrash due to a StrictMode violation :| |
cortinico
requested changes
Feb 25, 2023
There was a problem hiding this comment.
Thanks for sending this over @ArjanSM
I don't think we want to evolve the API in this direction as this will expose the Request object from OkHTTP to the user.
A user can manipulate the request (i.e. Read the body) in way we cannot prevent.
I think the right way to approach this feature is either:
- Provide a list of paths to use for the skip (a
List<String>) - Provide a list of header to strip and use for the skip (again a
List<String>)
library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt
Outdated
Show resolved
Hide resolved
- adds example in sample app. - adds tests
cortinico
reviewed
Mar 4, 2023
library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt
Outdated
Show resolved
Hide resolved
library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt
Outdated
Show resolved
Hide resolved
sample and tests updated accordingly.
cortinico
requested changes
Mar 9, 2023
library/src/test/kotlin/com/chuckerteam/chucker/util/ChuckerInterceptorDelegate.kt
Outdated
Show resolved
Hide resolved
library/src/test/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptorTest.kt
Outdated
Show resolved
Hide resolved
library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt
Outdated
Show resolved
Hide resolved
cortinico
changed the title
skipPaths to selectively skip requests from Chucker
cortinico
approved these changes
Apr 8, 2023
Comment on lines
+85
to
+87
| return if(shouldProcessTheRequest) | ||
| responseProcessor.process(response,transaction) | ||
| else response |
There was a problem hiding this comment.
Let's merge it as it is, but I believe KtLint is broken here as this is not properly formatted
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📄 Context
Fixes #266
📝 Changes
I added a public API,
skipPaths(paths:List<String>), to theChuckerInterceptor.Builer.The ChuckerInterceptor decides to process the transaction based on the outcome of the lambda in
skipEndpoints.🚫 Breaking
Adding
skipPaths(..)to theChuckerInterceptor.Buildwill be a breaking change.🛠️ How to test
ChuckerInterceptorTest/anythingto the HttpBinHttpTask.kt and added/anythingto the skip paths list in the sample app.