JeringTech / Javascript.NodeJS

Invoke Javascript in NodeJS, from C#
Other
463 stars 43 forks source link

App crash on large number of errors #54

Closed DaniilSokolyuk closed 4 years ago

DaniilSokolyuk commented 4 years ago

When a heavy load and a large number of errors from node.js, the application crashes with an error

Index was outside the bounds of the array.
   at System.String.get_Chars(Int32 index)
   at Jering.Javascript.NodeJS.OutOfProcessNodeJSService.TryCreateMessage(StringBuilder stringBuilder, String data, String& message)
   at Jering.Javascript.NodeJS.OutOfProcessNodeJSService.ErrorDataReceivedHandler(Object sender, DataReceivedEventArgs evt)
   at System.Diagnostics.Process.ErrorReadNotifyUser(String data)
   at System.Diagnostics.AsyncStreamReader.FlushMessageQueue(Boolean rethrowInNewThread)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Diagnostics.AsyncStreamReader.<>c.<FlushMessageQueue>b__18_0(Object edi)
   at System.Threading.QueueUserWorkItemCallback.<>c.<.cctor>b__6_0(QueueUserWorkItemCallback quwi)
   at System.Threading.ExecutionContext.RunForThreadPoolUnsafe[TState](ExecutionContext executionContext, Action`1 callback, TState& state)
   at System.Threading.QueueUserWorkItemCallback.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
JeremyTCD commented 4 years ago

Do you know what your JS is writing to console?

TryCreateMessage processes stdout, accumulating characters until '\0':

https://github.com/JeringTech/Javascript.NodeJS/blob/fbd57f7633ab0aef831eb69bf433c4610ad1fa50/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs#L320-L339

https://github.com/JeringTech/Javascript.NodeJS/blob/fbd57f7633ab0aef831eb69bf433c4610ad1fa50/src/NodeJS/NodeJSServiceImplementations/OutOfProcess/OutOfProcessNodeJSService.cs#L281-L305

Will try to reproduce in unit tests tomorrow, if you know what your applications is writing to stdout (console.log(...)) or you can debug and check what the data argument is on crashes it'll help.

Edit:

Check if data is ever an empty string. Offhand, the only time data[data.Length - 1] might throw an index out of range exception is if data.Length == 0, in which case we'd be passing -1 to the indexer, data[-1].

TODO tomorrow:

value can't be a string when the exception is thrown, because if it is, final length >= 2. Therefore console.log(null) or console.log(undefined) is being called. Check logging of objects too, console.log(obj), do we need to call toString and manually add null terminator?

JeremyTCD commented 4 years ago

I've updated my previous comment, I think calling console.log(undefined) or console.log(null) may cause the error. Need to test and add guards.

DaniilSokolyuk commented 4 years ago

@JeremyTCD Some error from workerpool library when terminate pool, data is empty image

JeremyTCD commented 4 years ago

Thanks for the extra info. #55 handles the case where data is an empty string.

I'd still like to figure out why empty strings are being written to stdout/stderr by Node.js. Turns out console.log(null), console.log(undefined) and console.log(obj) work fine.

Could you share how I can reproduce your situation?

JeremyTCD commented 4 years ago

Turns out data is empty when log messages contain an empty line, e.g console.log('a\n\nb'). TryCreateMessage wasn't handling empty lines properly.

In your case, most probably some library called console.error with a detailed message containing empty lines for formatting.

Thanks for reporting this bug. I've released v4.4.1 with the fix. Feel free to close this issue if it resolves the problem for you.

DaniilSokolyuk commented 4 years ago

@JeremyTCD thank you for fix, issue resolved