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

change to signature of JWT.decode method #14

Closed
rwygand opened this issue Feb 12, 2013 · 5 comments
Closed

change to signature of JWT.decode method #14

rwygand opened this issue Feb 12, 2013 · 5 comments

Comments

@rwygand
Copy link

rwygand commented Feb 12, 2013

JWT.decode can now, since the inclusion of MultiJson, raise a MultiJson::Decode error from lines 67 and 68. This means that he method signature of decode has changed to raise both JWT::DecodeError and MultiJson::DecodeError. Was this change intentional? It's breaking some test cases of mine that expect a JWT::DecodeError to be raised.

@progrium
Copy link
Contributor

I don't believe it's intentional. I assume you'd expect the
MultiJson::DecodeError to be caught and re-raised as JWT::DecodeError? That
sounds fine to me if you want to patch.

On Tue, Feb 12, 2013 at 10:02 AM, Rob Wygand notifications@github.comwrote:

JWT.decode can now, since the inclusion of MultiJson, raise a
MultiJson::Decode error from lines 67 and 68. This means that he method
signature of decode has changed to raise both JWT::DecodeError and
MultiJson::DecodeError. Was this change intentional? It's breaking some
test cases of mine that expect a JWT::DecodeError to be raised.


Reply to this email directly or view it on GitHubhttps://github.com//issues/14.

Jeff Lindsay
http://progrium.com

@rwygand
Copy link
Author

rwygand commented Feb 12, 2013

Yeah, that's what I figured... I just wanted to be sure of intent. I'll
submit a patch ASAP.

On Tue, Feb 12, 2013 at 12:26 PM, Jeff Lindsay notifications@github.comwrote:

I don't believe it's intentional. I assume you'd expect the
MultiJson::DecodeError to be caught and re-raised as JWT::DecodeError?
That
sounds fine to me if you want to patch.

On Tue, Feb 12, 2013 at 10:02 AM, Rob Wygand notifications@github.comwrote:

JWT.decode can now, since the inclusion of MultiJson, raise a
MultiJson::Decode error from lines 67 and 68. This means that he method
signature of decode has changed to raise both JWT::DecodeError and
MultiJson::DecodeError. Was this change intentional? It's breaking some
test cases of mine that expect a JWT::DecodeError to be raised.


Reply to this email directly or view it on GitHub<
https://github.com/progrium/ruby-jwt/issues/14>.

Jeff Lindsay
http://progrium.com


Reply to this email directly or view it on GitHubhttps://github.com//issues/14#issuecomment-13455314.

@progrium
Copy link
Contributor

progrium commented Apr 1, 2013

Did you submit a patch for this?

@rwygand
Copy link
Author

rwygand commented Apr 2, 2013

Yes, and it was merged: #16

@progrium
Copy link
Contributor

progrium commented Apr 2, 2013

Cool.

@progrium progrium closed this as completed Apr 2, 2013
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

No branches or pull requests

2 participants