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

fix: #298: register on close and error event on response #299

Merged
merged 1 commit into from
Feb 11, 2022
Merged

fix: #298: register on close and error event on response #299

merged 1 commit into from
Feb 11, 2022

Conversation

giovanni-bertoncelli
Copy link
Contributor

Resolves #298.
Added handlers to handle error and close events on response and resolve/reject respectively the context caller.

@giovanni-bertoncelli giovanni-bertoncelli changed the title fix: register on close and error event on response fix: #298: register on close and error event on response Feb 11, 2022
@icebob
Copy link
Member

icebob commented Feb 11, 2022

Great, thanks!

@icebob icebob merged commit c6f16c6 into moleculerjs:master Feb 11, 2022
@giovanni-bertoncelli
Copy link
Contributor Author

giovanni-bertoncelli commented Feb 14, 2022

@icebob maybe the call should be always resolved instead of rejected since if rejected moleculer tries to write the headers, but since the response stream has already started the headers are surely already been written. Which approach do you prefer?

@icebob
Copy link
Member

icebob commented Feb 19, 2022

Can we check that the response stream is started? And in this case, we block the header sending.

@giovanni-bertoncelli
Copy link
Contributor Author

@icebob how can we do that? By resolving if headersSent is true? And rejecting otherwise?

(BTW @michelevincenzi is my colleague, we're working together on this)

@icebob
Copy link
Member

icebob commented Feb 25, 2022

we can reject, but check the headersSent value in sendError.

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.

Possible memory leak on alias with custom function on error
2 participants