Skip to content

Conversation

@iamdanfox
Copy link
Contributor

@iamdanfox iamdanfox commented Sep 10, 2018

Before this PR

asConjure would return a broken result if a server returns json with both remoting2 and remoting2 types, e.g.:

{
    "errorCode":"CONFLICT",
    "errorName":"Product:TransactionConflict",
    "errorInstanceId":"7d345390-e41d-49dd-8dcb-38dd716c0347",
    "parameters":{},
    "exceptionClass":"CONFLICT",
    "message":"Refer to the server logs with this errorInstanceId: 7d345390-e41d-49dd-8dcb-38dd716c0347"
}

It would translate this into the broken:

{
    "errorCode":"CONFLICT",
    "errorName":"Refer to the server logs with this errorInstanceId: 7d345390-e41d-49dd-8dcb-38dd716c0347",
    "errorInstanceId":"7d345390-e41d-49dd-8dcb-38dd716c0347",
    "parameters":{}
}

After this PR

It translates it into the correct SerializableError.

Open questions

  • how can we actually publish this, now that our bintray creds point to the wrong place???

@iamdanfox iamdanfox requested a review from a team as a code owner September 10, 2018 15:08
@iamdanfox iamdanfox changed the base branch from develop to release/1.x September 10, 2018 15:08
}

@Test
public void asConjure_converts_json_with_both_remoting2_and_remoting3_fields() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also include parameters and errorInstanceId?

@qinfchen
Copy link
Contributor

qinfchen commented Sep 10, 2018

how can we actually publish this, now that our bintray creds point to the wrong place???

can we do what we did with the new maven coordinates before the switch?

Copy link
Contributor

@qinfchen qinfchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just minor comments

@iamdanfox
Copy link
Contributor Author

My plan for publishing is to temporarily rename the repo from conjure-java-runtime-api -> http-remoting-api re-do the bintray tokens, tag a release, then rename back to conjure-java-runtime-api and reset the tokens.

@qinfchen
Copy link
Contributor

👍

@iamdanfox iamdanfox merged commit c11640c into release/1.x Sep 11, 2018
@iamdanfox iamdanfox deleted the fix-asConjure-method branch September 11, 2018 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants