-
Notifications
You must be signed in to change notification settings - Fork 221
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
Whhtml driver timeout #229
base: master
Are you sure you want to change the base?
Conversation
…Context references to ensure AsResultBase.BuildFile doesn't break when being called from async/await code.
…e or code from the wkhtmltopdf-process
Replace references to HttpContext.Current with ControllerContext Http…
…-reinhold Fix for issue webgio#189 on webio/Rotativa
* currently whtml's doesn't have parameter to set timeout time. We provide this functionality is Rotativa. When starting process of whtml (pdf or image) we can wait proces exit for given amout of time. See issue webgio#203
var pdfTester = new PdfTester(); | ||
pdfTester.LoadPdf(pdfBinary); | ||
pdfTester.PdfIsValid.Should().Be.True(); | ||
pdfTester.PdfContains("Rotativa").Should().Be.True(); | ||
} | ||
|
||
[Test] | ||
public void Can_build_the_image_binary() | ||
public void Failed_to_build_the_pdf_binary_when_timeout_exceed() |
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.
some code refactor & create tests for checking timeout's
Rotativa/Rotativa.nuspec
Outdated
@@ -2,7 +2,7 @@ | |||
<package > | |||
<metadata> | |||
<id>Rotativa</id> | |||
<version>1.7.3</version> | |||
<version>1.8.0</version> |
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 decided it is Minor, cuz I've added backwards compatible functionality to the public API (ConvertTimeout which is not required)
See https://semver.org/spec/v2.0.0.html#spec-item-7
else | ||
{ | ||
var cancellationTokenSource = new CancellationTokenSource(); | ||
var task = Task.Run(() => ConvertExecute(wkhtmlPath, ref switches, ref html, wkhtmlExe, cancellationTokenSource.Token)); |
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 is not as I could imagine best, but it works.
I've tried to do it by calling
proc.WaitForExit(milliseconds: timeeout)
but this requires that output of process is asynchronous.
I've tried to do it also by implementing events:
proc.OutputDataReceived += (sender, outputLine) => ...
proc.ErrorDataReceived += (sender, errorLine) => ...
but I failed to convert outputLine.Data to bytes[]. Thats cuz it is String, not Stream (as in proc.StandardOutput.BaseStream).
I've tried provide some encoding to process output and convert it, but any way I tried to do it, the resulted PDF was broken.
Finally I gave up and it that way
Rotativa/WkhtmlDriver.cs
Outdated
cancellationToken.Register(() => | ||
{ | ||
if (!proc.HasExited) | ||
proc.Kill(); |
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.
need to kill manually process when timeouted
Description
Currently whtml's doesn't have parameter to set timeout time. i've provide this functionality is Rotativa. When starting process of whtml (pdf or image) we can wait proces exit for given amout of time.
Additional info
I've also accepted some code from PR in original repository:
Fixes
This PR fixes issue #203