-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add auto-flushing to middleware integration #2017
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
Conversation
packages/node/src/handlers.ts
Outdated
|
||
res.end = async function end (chunk?: any, encodingOrCb?: string | Function, cb?: Function) { | ||
await flush(2000) | ||
console.log('flushed') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug leftovers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry I've already removed it
packages/node/src/handlers.ts
Outdated
@@ -221,12 +222,21 @@ export function requestHandler(options?: { | |||
transaction?: boolean | TransactionTypes; | |||
user?: boolean | string[]; | |||
version?: boolean; | |||
wait?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this number | undefined
instead of a boolean
and rename it to flushTimeout
.
Also, use flushTimeout
then instead of the hardcoded 2000
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then maybe make it number
and let 0
be the default which means "no flush"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would also work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool stuff, thx 🥇
As per request I send PR implementing #2001
I've checked manually that it indeed flushes errors before request finishes if { wait: true } is used
Unfortunately there's no testing scaffold for existing code so it's hard for me to add new