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

Added more error handling #7

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Added more error handling #7

wants to merge 8 commits into from

Conversation

nepda
Copy link

@nepda nepda commented Jan 28, 2016

A few more messages

@mbonneau
Copy link
Member

Hi @nepda ,

I made some changes after the PR from early and added some automated testing as well.

Is there any way that you can rebase these changes on the current release (0.1.1)?

@nepda
Copy link
Author

nepda commented Jan 29, 2016

You've changed the error handling from returning the error to the poster, to an "internal" exception. Can you explain why this is the better approach?
In my use case, I want to get informed from the "client" point of view.

@nepda
Copy link
Author

nepda commented Jan 29, 2016

I will fix the broken tests as soon as the error handling question is clarified ;)

@mbonneau
Copy link
Member

In the test:
f9fbcb0#diff-8f3b5fbe3ee084c8a46e4bd7661d3297R24
I am just reusing the WampPost client as the Callee end also. So the test is structured like:

 +-----------+
 |HTTP Client|
 +-----------+
       ^
       |
     HTTP
       |
       v
  +--------+
  |WampPost|<---+
  +--------+    |
       ^      WAMP
       |        |
     WAMP       |
       |        |
       v        |
  +-------+     |
  |Thruway+<----+
  |Router |
  +-------+

With Thruway, you have to throw an exception to send an error back from the Callee side. Throwing the WampErrorException gives that end control over exactly what gets put into the error message.

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.

2 participants