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

feat(integration): add fastify to the integration test suite #529

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

domharrington
Copy link
Member

🧰 Changes

Building on the work over in #528 - adding fastify to the integration test suite.

🧬 QA & Testing

Do the tests pass? ✅

@domharrington domharrington force-pushed the feature/fastify-integration-test branch from d76fb46 to c093e21 Compare August 10, 2022 11:48
@@ -21,16 +34,16 @@ fastify.addHook('onSend', async (request, reply, payload) => {

startedDateTime: new Date(request.readmeStartTime),
responseEndDateTime: new Date(),
responseBody: payload,
responseBody: reply.payload,
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference between the payload in onResponse hooks versus the one in onSend?

Copy link
Member Author

@domharrington domharrington Aug 10, 2022

Choose a reason for hiding this comment

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

Yeah great question, and it took me a good couple of hours to figure out the right hook to use here. onSend is called right before any response has been returned, so the data is there but there's no content-length header set up yet, so the har file has a content.size of 0:

size: Number(fixHeader(res.getHeader('content-length') || 0)),

onResponse is called after the response has been sent, but you can't access the body at that point... hence why we've gotta attach the payload in the onSend, then process it after the response has gone down.

The reason why we have to pass request.raw to the log function is because fastify's Request wrapper doesn't have request.httpVersion so this was coming through as HTTP/undefined.

The reason why we have to pass the Reply instance to the log function (as opposed to reply.raw) is also nuanced, but basically Node's ServerResponse object removes all headers from the object after it's been written to the response, so res.getHeader('content-length') would return with undefined when the response has finished: nodejs/node#10354

I hope this makes sense 😅 it didn't make a ton of sense to me tbh, hence why i'm hoping to modify the API design to make all of this stuff a little easier to figure out.

@erunion erunion merged commit ffc13a6 into main Aug 10, 2022
@erunion erunion deleted the feature/fastify-integration-test branch August 10, 2022 16:46
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