Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Implementing OnResponseCompleted #64

Merged
merged 1 commit into from
Mar 17, 2015
Merged

Implementing OnResponseCompleted #64

merged 1 commit into from
Mar 17, 2015

Conversation

ajaybhargavb
Copy link
Contributor

@ghost
Copy link

ghost commented Feb 20, 2015

Hi @ajaybhargavb, I'm your friendly neighborhood Microsoft Open Technologies, Inc. Pull Request Bot (You can call me MSOTBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Ajay Bhargav Baaskaran). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla2.msopentech.com.

TTYL, MSOTBOT;

@ghost ghost added the cla-not-required label Feb 20, 2015
davidfowl
davidfowl reviewed Mar 5, 2015
View reviewed changes
src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs
@@ -303,6 +341,8 @@ public void ProduceEnd(Exception ex)
{
ProduceStart();

FireOnResponseCompleted();
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this is in the right place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved.

lodejard
lodejard reviewed Mar 16, 2015
View reviewed changes
src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs
}
catch
{
// Ignore exceptions
Copy link
Contributor

Choose a reason for hiding this comment

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

We should log a bug to figure out how to get the ILoggerFactory to the server. Doesn't need to block this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created an issue. #81

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also in Helios. https://github.com/aspnet/Helios/issues/117

@lodejard
Copy link
Contributor

:shipit:

@ajaybhargavb ajaybhargavb merged commit c1e298b into dev Mar 17, 2015
@halter73 halter73 deleted the response-feature branch August 19, 2015 18:41
@yuwaMSFT yuwaMSFT mentioned this pull request Feb 25, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ajaybhargavb @lodejard @davidfowl