Refactor to exceptions #31
Open
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
emhoracek
commented
Jan 24, 2017
| @@ -204,7 +206,7 @@ clearRedisCache ctxt = R.runRedis (_redis ctxt) (rdelstar "wordpress:*") | |||
|
|
|||
| unobj :: Value -> Object | |||
| unobj (Object x) = x | |||
| unobj _ = error "Not an object" | |||
| unobj _ = throw NotAnObject | |||
There was a problem hiding this comment.
this is silly, should've just stayed an error (it's only for testing)
emhoracek
commented
Jan 24, 2017
| ("custom-endpoint-array", ctxt) `shouldRenderContaining` "2014-10-01" | ||
| ("custom-endpoint-array", ctxt) `shouldRenderContaining` "2014-10-02" | ||
| ("custom-endpoint-array", ctxt) `shouldRenderContaining` "2014-10-15" | ||
| it "should be able to reference fields from the custom endpoint in another custom endpoint query" $ do | ||
| ("custom-endpoint-array", ctxt) `rendersSameAs` "custom-endpoint-enter-the-matrix" | ||
| it "should handle 404s without blowing up" $ | ||
| ("custom-endpoint-404", ctxt) `shouldRenderContaining` "404" |
There was a problem hiding this comment.
This works, but the wreq/http-client exception has a LOT of information that doesn't belong in the HTML debugging comment. Right now all the information about the request is in there.
emhoracek
commented
Jan 24, 2017
| where getAndCache = do jsonBlob <- wpRequest wpKey | ||
| wpCacheSet wpKey jsonBlob | ||
| stopReqMutex wpKey | ||
| return jsonBlob |
There was a problem hiding this comment.
this is so much easier to look at! yay!
emhoracek
commented
Jan 24, 2017
|
|
||
| errorUnless :: Text -> IO (CacheResult a) -> IO (Either StatusCode a) | ||
| catch action (\(e :: CacheInProgress) -> CC.threadDelay 100000 >> retryUnless action) | ||
|
|
There was a problem hiding this comment.
So here I'm caching this CacheInProgress exception, but I'm not actually throwing it in startMutex yet
(There are no tests that exercise this stuff!)
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.
Tests currently pass, but still some work to do. Would appreciate comments, suggestions.
It seems like I ought to be able to pull out the "log and comment" parts of the exception handling into a function, but I'm not sure how yet.
I like that the parts that are throwing exceptions seem much simpler now, but all the complexity is just being offloaded to the Fills. :/
Biggest problem is that I made this
CacheInProgressexception that the retry function catches, but I'm not actually throwing it in the mutex function yet. So still need to figure that out.