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

Try.withCatch #256

Open
jainh opened this issue Mar 10, 2017 · 9 comments
Open

Try.withCatch #256

jainh opened this issue Mar 10, 2017 · 9 comments
Labels

Comments

@jainh
Copy link

jainh commented Mar 10, 2017

When i want to try like as follows
Try.witchCatch(()->a()).onFail().map(()->b())..
where b() throws an exception as well, it says b with unhandled exception, who do deal in such situation? we can not call from map or flatmap function that throws an exception?

@johnmcclean
Copy link
Member

johnmcclean commented Mar 10, 2017

Hey @jainh, this is the expected behaviour by default, although Try can be configured to catch subsequent exceptions in map / flatMap.

You can use Try.of(value,Exceptions..) to configure Try such that it will capture any Exceptions thrown in subsequent methods.

The reason we don't do this be default is because it may hide errors in code and we felt it important that engineers make a conscious / deliberate decision to use it.

Your example

 Try.witchCatch(()->a())
     .onFail(...)
     .map(v->b(v));

Could become

   Try.of(null, Throwable.class)
       .map(__>a())
       .onFail(...)
       .map(v->b(v));

We see Try primarily as a data type that represents either an error or a successful value, rather than being primarily a trapper of exceptions (although it can be used for that purpose). An alternative to turning on full exception catching, is to use flatMap with a nested Try.withCatch

  Try.witchCatch(()->a())
     .onFail(...)
     .flatMap(v->Try.withCatch(()->b(v)));

@jainh
Copy link
Author

jainh commented Mar 10, 2017

@johnmcclean-aol
I think i tried the last option , i just did now again, here is the snippet

 Try.withCatch(() -> decodeBase64(token))
                .onFail(e -> LOG.error("error in decoding base64 token string:{}",e))
                .flatMap(v -> Try.withCatch(()-> (JSONObject)new JSONParser().parse(v)))

But (JSONObject)new JSONParser().parse(v) inside flatmap show red swiggle line with unhandeled parse exception.

@johnmcclean
Copy link
Member

Hmmm.. That seems strange. Try.withCatch takes a CheckedSupplier so it should compile. What version are you using?

This compiles for me (tested against cyclops-react 2.0.0-MI4)

public int throwsEx() throws Exception{
    return 0;
}
@Test
public void testExcept(){
    assertThat(Try.withCatch(()-> throwsEx())
            .map(i->i+" woo!")
            .onFail(System.out::println)
            .flatMap(e->Try.withCatch(()-> throwsEx()))
            .orElse(1),is(0));


}

If you are using cyclops-react (rather than cyclops-try) you could also use ExceptionSoftener.softenSupplier to soften the ParseException (but you shouldn't have too).

@jainh
Copy link
Author

jainh commented Mar 10, 2017

<dependency>
			<groupId>com.aol.cyclops</groupId>
			<artifactId>cyclops-try</artifactId>
			<version>7.2.3</version>
		</dependency>

I think this could IDE intellij i am using but let me run it then i will update you.

@jainh
Copy link
Author

jainh commented Mar 10, 2017

@johnmcclean-aol do you how to apply parallel operation on result return after successful flatmap operation and then collect them ?

@johnmcclean
Copy link
Member

Please do. I think that version should compile also (as it is also using CheckedSupplier to handle Checked Exceptions e.g. https://github.com/aol/cyclops/blob/v7.2.0/cyclops-try/src/main/java/com/aol/cyclops/trycatch/Try.java#L224)

@johnmcclean
Copy link
Member

@jainh Do you want to process a the result in parallel in some way? i.e. is it a Collection that you would convert to a Stream and execute in parallel? I can show an example of that below

 Try.withCatch(() -> decodeBase64(token))
            .onFail(e -> LOG.error("error in decoding base64 token string:{}",e))
            .flatMap(v -> Try.withCatch(()-> (JSONObject)new JSONParser().parse(v)))
            .map(this::processInParallel); //I think it's better to define a method for this


public List<Type> processInParallel(List<JSONType> data){
     return data.stream()
                .parallel()
                .map(this::process)
                .collect(Collectors.toList());

}

@jainh
Copy link
Author

jainh commented Mar 10, 2017

  Try<Map<String, String>, Throwable> jsonString = Try.withCatch(() -> decodeBase64(token))
                .onFail(e -> LOG.error("error in decoding base64 token string:{}",e))
                .flatMap(v -> Try.withCatch(()-> (JSONObject)(new JSONParser()).parse(v)))
                .onFail(e -> LOG.error("error in parsing token string:{}",e))
                .map(v -> convertToMap(new ObjectMapper().readValue(v, HashMap.class)));

but value in map v is expected to be JSONObject but it shows R.
what i am trying to do : parse string to JSONObject convert it to Map and then to JWT map then to JWTClaimSet.

@jainh
Copy link
Author

jainh commented Mar 10, 2017

@johnmcclean-aol

  public JwtUserDto parseToken(String token) throws JwtTokenMalformedException {
        JwtUserDto u = null;
        Gson gson = new Gson();

       Map<String, Try<JWTClaimsSet, ParseException>> claimsSets = Try.withCatch(() -> decodeBase64(token))
                .onFail(e -> LOG.error("error in decoding base64 token string:{}",e))
                .flatMap(jsonString -> gson.fromJson(jsonString, new TypeToken<Map<String, Object>>(){}.getType()))
                .onFail(e -> LOG.error("error in parsing token string:{}",e))
                .map(jsonMap -> convertToMap((Map<String, Object>)jsonMap))
                .onFail(e -> LOG.error("error failed to convert to map of string -> string : {}",e))
                .map(tokenMap -> tokenMap.entrySet().stream()
                        .collect(Collectors.toMap(Map.Entry :: getKey, p -> convertToClaimsSet(p.getValue()))))
               .get();


       if(claimsSets != null && !claimsSets.isEmpty()) {
           Date today = new Date();
           u = new JwtUserDto();
           if(claimsSets.get("ACCESS_TOKEN").get().getExpirationTime().before(today)){
               throw new JwtTokenMalformedException("Token is expired ");
           }
           JWTClaimsSet idToken = claimsSets.get("ID_TOKEN").get();
           u.setId((String)idToken.getClaims().get("user_id"));
           u.setUsername(idToken.getSubject());
       }
        return u;
    }

    private Map<String, String> convertToMap(Map<String,Object> data){
        return data.entrySet()
                   .stream()
                   .filter(v -> Arrays.asList("ID_TOKEN","ACCESS_TOKEN").contains(v.getKey()))
                   .collect(Collectors.toMap(Map.Entry::getKey, p -> (String)p.getValue()));
    }

    private Try<JWTClaimsSet, ParseException> convertToClaimsSet(String jwtToken){
        Try<JWTClaimsSet, ParseException> claimsSets = Try.withCatch(() -> JWTParser.parse(jwtToken))
            .onFail(e -> LOG.error("failed to parser jwt token because of the following reason:{}", e))
            .flatMap(v -> Try.withCatch( () -> v.getJWTClaimsSet()))
            .onFail(e -> LOG.error("failed to parser jwt token because of the following reason:{}", e));
        return claimsSets;
    }

Does that look ok to you? just sanity check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants