fix bufio.Scanner: token too long

everesio commented 4 years ago

Description

Replace bufio.NewScanner by bufio.NewReaderSize as the scanner returns "token too long" error when the line is longer than 65536 bytes

Motivation and Context

When function receives and logs long input, the "bufio.Scanner: token too long" error is logged and the request processing is stopped.

[ ] I have raised an issue to propose this change (required)

How Has This Been Tested?

go test was added. function built with custom image everesio/of-watchdog:0.7.7-fix is not broken any more.

Types of changes

[X] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I've read the CONTRIBUTION guide
[x] I have signed-off my commits with git commit -s
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

alexellis commented 4 years ago

/add label: design/review

alexellis commented 4 years ago

/add label: not approved

alexellis commented 4 years ago

My thoughts are the same as what Lucas said initially, we need to understand the use-case here. Thanks for raising an issue, but it's still lacking some details about the problem being solved.

Alex

alexellis commented 4 years ago

Please hold off on further work. This PR is not approved and we have an open discussion on the issue.

everesio commented 4 years ago

I opened the https://github.com/openfaas-incubator/of-watchdog/issues/100 and added the test handler. Use case: ML functions receives json event longer than 64kb. The event is logged.

everesio commented 4 years ago

Hi @alexellis,

I am a little confused. How do you want to proceed with the PR or solving the issue in other way ?
The problem is use case independent i.e. log line longer than 64kb breaks the service.

Regards, Michal

alexellis commented 3 years ago

I'm trying to understand why there is a log line of 64kb. I'll take a look at your issue to see if it's been updated yet.

everesio commented 3 years ago

I'm trying to understand why there is a log line of 64kb.

Service logs a stack trace

It is not always possible to control log size coming from 3rd party libs

openfaas / of-watchdog