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

Hook socket message (documentation) inconsistencies #834

Closed
hobofan opened this issue Aug 1, 2017 · 6 comments
Closed

Hook socket message (documentation) inconsistencies #834

hobofan opened this issue Aug 1, 2017 · 6 comments

Comments

@hobofan
Copy link
Contributor

hobofan commented Aug 1, 2017

While implementing the hook handler for Rust I stumbled upon a few things that I feel are mis-/underdocumented. I'll write up a PR for it, but I'd like to get some input on it first.

The documentation under https://github.com/apiaryio/dredd/blob/6445caa0976eaf83b460bf7eade7ed73843845de/docs/hooks-new-language.md#tcp-socket-message-format shows a few event types that don't actually exist (or are not sent) like before and after and instead have to be triggered by some application logic in the hook handler. I think that should be mentioned by the documentation.

The data field can either be an array or an object. I have seen that it is only an array for beforeAll and afterAll events, but never have seen more than one element in it. Are there actually any circumstances under which there might be more than element in it?

@honzajavorek
Copy link
Contributor

@hobofan Thanks for all the work and the investigation. I need to take a look at the before and after events, because it sounds suspicious to me that they are not sent. I mean - it might be a bug in Dredd, not in docs 😱 I need to verify that part.

Regarding beforeAll and afterAll, they get an array of all transactions in the API description document. If you tested with just one endpoint, you get an array of a single transaction.

@hobofan
Copy link
Contributor Author

hobofan commented Aug 7, 2017

I mean - it might be a bug in Dredd, not in docs 😱 I need to verify that part.

I took that bit of arcane knowledge from the golang implementation.

Fixing that bug would very certainly break the Go and Rust implementations. Considering that this would be another breaking change besides #712 (comment), I feel that there is a need for introducing versioning of the hook handling API to allow for a more robust API evolvement in the future, but thats a topic for its own issue... 😅

@honzajavorek
Copy link
Contributor

I took that bit of arcane knowledge from the golang implementation.

I've just realized that in the time I was learning about Dredd, there were some issues from @snikch filed, which addressed certain things which came up during his work on the Go hooks. Digged them up and found #271, which sounds completely relevant to this issue.

The hook handling API should certainly get more attention. There are multiple issues to be addressed, some work in progress, ... Need to find time to tidy up there.

@honzajavorek
Copy link
Contributor

@hobofan If the second part of your issue about beforeAll & afterAll and array of transactions is resolved, would you mind if I merge this with #271?

BTW I found also #269, where the architecture is discussed as well.

@hobofan
Copy link
Contributor Author

hobofan commented Aug 8, 2017

I saw #269, but didn't find #271, which is definitely a duplicate, so I'll close this now.

@hobofan hobofan closed this as completed Aug 8, 2017
@honzajavorek
Copy link
Contributor

Thanks! 👍 I think this is very important confirmation of the issue and it should be getting attention together with polishing the hook handler API and hook handlers' integration test suite.

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

No branches or pull requests

2 participants