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

Routes are not being correctly matched by method #173

Closed
thosakwe opened this Issue Oct 3, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@thosakwe
Copy link
Member

thosakwe commented Oct 3, 2017

No description provided.

@thosakwe

This comment has been minimized.

Copy link
Member

thosakwe commented Oct 4, 2017

Turned out to be a caching issue.

@Rodsevich

This comment has been minimized.

Copy link

Rodsevich commented Nov 10, 2017

And how did you solve it?
I'm having a very similar one:
start server
GET /api/service
200 []
POST /api/service {"foo":"bar}
200 []

However if I restart and invert the order...
POST /api/service {"foo":"bar}
201 {id:1,foo:"bar",created...}
GET /api/service
201 {id:1,created...}

@thosakwe

This comment has been minimized.

Copy link
Member

thosakwe commented Nov 10, 2017

Version 1.0.8 and beyond is broken in that regard. The bug has been fixed in 1.1.0-alpha; however, that comes with some changes.

I will have to clone this separately on my computer, revert back in the history to 1.0.10, and then patch it with a hot fix of sorts. It'll be 1.0.11, with no new features - just bugfixes for the existing infrastructure. I'll get this out within 24 hours.

@thosakwe

This comment has been minimized.

Copy link
Member

thosakwe commented Nov 10, 2017

Basically, there is a "handlerCache" that maps request paths to resolved routes, to save processing power. However, the original implementation didn't keep the request method in mind, so that's what causes this bug. Pesky, I know.

The worst part is that this escaped tests. In the routing tests, a new server is created for every test, instead of sharing one instance, so there wasn't a check to make sure this wasn't in place.

@thosakwe

This comment has been minimized.

Copy link
Member

thosakwe commented Nov 28, 2017

@Rodsevich Sorry I took way too long to get to this, but the bug is patched in package:angel_framework@1.0.11.

@Rodsevich

This comment has been minimized.

Copy link

Rodsevich commented Nov 29, 2017

Thank you so much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment