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

JBPM-9942 When try to attach a document the file can not greater tha… #2648

Merged
merged 1 commit into from May 28, 2022

Conversation

bxf12315
Copy link
Member

…n 2 MB

Thank you for submitting this pull request

JIRA: (please edit the JIRA link if it exists)

[link](JBPM-9942)

referenced Pull Requests: (please edit the URLs of referenced pullrequests if they exist)

  • paste the link(s) from GitHub here
  • link 2
  • link 3 etc.
How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

How to retest this PR or trigger a specific build:
  • a pull request please add comment: Jenkins retest this

  • a full downstream build please add comment: Jenkins run fdb

  • a compile downstream build please add comment: Jenkins run cdb

  • a full production downstream build please add comment: Jenkins execute product fdb

  • an upstream build please add comment: Jenkins run upstream

@bxf12315
Copy link
Member Author

Jenkins run fdb

@sonarcloud
Copy link

sonarcloud bot commented Nov 10, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@prashuts
Copy link

Jenkins run fdb

1 similar comment
@bxf12315
Copy link
Member Author

Jenkins run fdb


fileData.get(input.id).data = srcData;
fileReader.readAsDataURL(fileToLoad);
// console.log("Converted Base64 version is " + srcData);
Copy link
Contributor

Choose a reason for hiding this comment

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

@bxf12315 Could you please remove this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pefernan The code created by Maciej, if srcaData is very huge, the cost is very high, what do you think we remove it?

Copy link
Member

Choose a reason for hiding this comment

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

@bxf12315 honestly I'd keep it as it is for now (or with small changes) since right now the idea is to post the docs content as a part of a json object. So we don't have much more options than reading the doc content here.
I think the general assessment on forms is that they are not inteded to upload heavy documents, so I'd say that this is ok keep it like this. If there's a need to attach big documents, I'd recommend other strategies than using a form.

Choose a reason for hiding this comment

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

Back to the first message in this thread - please remove the commented line.

@nmirasch
Copy link
Contributor

@pefernan could you take a look when you have a chance?Thanks!

@pefernan
Copy link
Member

@jstastny-cz please take a look too :P

Copy link
Member

@pefernan pefernan left a comment

Choose a reason for hiding this comment

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

I'm ok with the change, I'd say that it's reasonable to wait for the file reader to finish the doc upload before converting the content into a dataurl. I'm pretty sure that we cannot do much more for large docs. However, I cannot make this work locally. I built an example with docs, the upload seem to work, but the filereader isn't reading the file content and submitting the form always ends with an empty doc. I'd say this is not a problem introduced by this change, but I think this should be addressed.

@sonarcloud
Copy link

sonarcloud bot commented Feb 11, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@bxf12315
Copy link
Member Author

Jenkins retest this

@bxf12315
Copy link
Member Author

Jenkins run fdb

@mareknovotny
Copy link
Member

jenkins retest this please

@mareknovotny
Copy link
Member

@nmirasch @pefernan @jstastny-cz please review

@mareknovotny mareknovotny dismissed pefernan’s stale review May 12, 2022 12:37

there were update on the PR

Copy link
Member

@pefernan pefernan left a comment

Choose a reason for hiding this comment

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

@bxf12315 good! it works fine, I added a comment with something I'd change (removing a console.log) that would make this run faster, but I'd consider this optional.

Thanks!

@mareknovotny
Copy link
Member

jenkins retest this please

@jstastny-cz
Copy link

Jenkins retest this

@jstastny-cz
Copy link

When I am attaching any file (even small in size), then pass it to a Human Task and try to download, the download does not start and server log shows:

11:33:16,166 ERROR [org.jbpm.workbench.pr.backend.server.ProcessDocumentServlet] (default task-8) Error occured during document retrieval "Unexpected HTTP response code when requesting URI 'http://localhost:8080/kie-server/services/rest/server/documents/d964c9d6-955e-454b-bf4e-cdc9811347b6'! Error code: 404, message: "Document with id d964c9d6-955e-454b-bf4e-cdc9811347b6 not found"".

When trying with big file, the weird thing to me is that the opload occurs instantly, so wondering if the saving actually works.

@jstastny-cz
Copy link

When I am attaching any file (even small in size), then pass it to a Human Task and try to download, the download does not start and server log shows:

11:33:16,166 ERROR [org.jbpm.workbench.pr.backend.server.ProcessDocumentServlet] (default task-8) Error occured during document retrieval "Unexpected HTTP response code when requesting URI 'http://localhost:8080/kie-server/services/rest/server/documents/d964c9d6-955e-454b-bf4e-cdc9811347b6'! Error code: 404, message: "Document with id d964c9d6-955e-454b-bf4e-cdc9811347b6 not found"".

When trying with big file, the weird thing to me is that the opload occurs instantly, so wondering if the saving actually works.

This was due to not having properly configured marshaller. Moving further.

@mareknovotny
Copy link
Member

@jstastny-cz please approve this too before merging.

@mareknovotny
Copy link
Member

the error happened in jacoco instrumentation build after the green one of build with tests

@mareknovotny
Copy link
Member

jenkins retest this please

@jstastny-cz
Copy link

@mareknovotny we're discussing issue with @bxf12315, I am seeing a NPE with a simple process.

@jstastny-cz
Copy link

Server log provides:

org.kie.server.api.exception.KieServicesHttpException: Unexpected HTTP response code when requesting URI 'http://localhost:8080/kie-server/services/rest/server/containers/jbpm9942_1.0/processes/jbpm9942.P1/instances'! Error code: 500, message: "Unable to create response: [jbpm9942.P1:5 - Task:3] -- null"
        at deployment.business-central.war//org.kie.server.client.impl.AbstractKieServicesClientImpl.createExceptionForUnexpectedResponseCode(AbstractKieServicesClientImpl.java:667)
        at deployment.business-central.war//org.kie.server.client.impl.AbstractKieServicesClientImpl.makeHttpPostRequestAndCreateCustomResponse(AbstractKieServicesClientImpl.java:364)
        at deployment.business-central.war//org.kie.server.client.impl.AbstractKieServicesClientImpl.makeHttpPostRequestAndCreateCustomResponse(AbstractKieServicesClientImpl.java:344)
        at deployment.business-central.war//org.kie.server.client.impl.ProcessServicesClientImpl.startProcess(ProcessServicesClientImpl.java:309)
        at deployment.business-central.war//org.jbpm.workbench.forms.display.backend.FormRendererProxyServlet.doPost(FormRendererProxyServlet.java:111)

Root cause:

Caused by: java.lang.NullPointerException
    at deployment.kie-server.war//org.apache.commons.io.FileUtils.writeByteArrayToFile(FileUtils.java:2843)
    at deployment.kie-server.war//org.apache.commons.io.FileUtils.writeByteArrayToFile(FileUtils.java:2826)
    at deployment.kie-server.war//org.jbpm.document.service.impl.DocumentStorageServiceImpl.saveDocument(DocumentStorageServiceImpl.java:87)
    at deployment.kie-server.war//org.jbpm.document.marshalling.DocumentMarshallingStrategy.marshal(DocumentMarshallingStrategy.java:68)
....

@bxf12315
Copy link
Member Author

@mareknovotny we're discussing issue with @bxf12315, I am seeing a NPE with a simple process.

The PR has a bug, I need to do in-depth research.

@bxf12315
Copy link
Member Author

@jstastny-cz please check it. Thanks

@mareknovotny
Copy link
Member

mareknovotny commented May 18, 2022

failed test in all runs GHA ans Jenkins too in org.kie.server.client.LoadBalancerClientTest.testDefaultLoadBalancerNoServersAvailable No available endpoints found

@jstastny-cz
Copy link

Jenkins retest this please
The built artifacts are not available anymore.

@jstastny-cz
Copy link

Jenkins retest this

Copy link
Member

@pefernan pefernan left a comment

Choose a reason for hiding this comment

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

+1 thanks @bxf12315

@sonarcloud
Copy link

sonarcloud bot commented May 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

@jstastny-cz jstastny-cz left a comment

Choose a reason for hiding this comment

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

LGTM.
With sufficient undertow post-size limit the ultimate boundary we have is the Java heap space. In my settings I've managed to overcome that limit and resulted in out of memory error. Which is expected and behaves same in BC or kie-server forms.

@mareknovotny mareknovotny merged commit 8be4b0a into kiegroup:main May 28, 2022
bxf12315 added a commit to Cory-jbpm/droolsjbpm-integration that referenced this pull request Jun 14, 2022
bxf12315 added a commit to Cory-jbpm/droolsjbpm-integration that referenced this pull request Jul 22, 2022
@mareknovotny
Copy link
Member

have you back ported to 7.67.x and 7.67.x-blue branches? @bxf12315

@bxf12315
Copy link
Member Author

have you back ported to 7.67.x and 7.67.x-blue branches? @bxf12315

back port PR in here https://github.com/kiegroup/droolsjbpm-integration/pull/2807/commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants