Upgrade Error JSTranslater

[aderosso]

We're using SmartGWT Enterprise 13.0-p20230212.

When we try to upgrade to version 13.0-p20230818 (also tried 13.0-p20230927), with no other changes, we're now getting an exception during an import from the BatchUploader. I've attached the stack trace as it's pretty big. We're also getting a lot of these warnings for many different classes:

WARN  com.isomorphic.js.JSTranslater:227 -   - org.eclipse.jetty.server.Server contains a (potentially indirect) looping reference to itself.  Returning null for recursed value.

For the code that processes the "add" form the BatchUploader we have written custom DMI that goes:

    public DSResponse executeAdd(final DSRequest req) throws Exception {
        final int size = req.getRPCManager().getRequests().size();

        final Map<String, Object> job = getJobSQLService().addJob(OpeningConstants.IMPORT_OPENINGS, BatchJobStatusConstants.IN_PROGRESS, size, 0, 0, 0, null);
        final long jobId = (long) job.get(DSConst.JobSQL.JOB_ID);

        final DSResponse res = new DSResponse(SmartGWTStatusCode.STATUS_SUCCESS);
        res.setData(job);

        /* Skipping all other request of queue. Process first request from queue and extract all the request from RPCManager. */
        req.getRPCManager().skipRemainingQueue();

        getImportOpeningService().startImport(req, jobId); // Async method to continue processing import

        return res;
    }

This is done because our import takes awhile to process, the work is done in an async process.

We've had this code like this running great for years, but now with the version upgrade the response from the queue of "add" requests is not working. Has something changed in a recently that would make this scenario not work?

[Isomorphic]

The BatchUploader operates in two phases: first, a CSV file is uploaded, then, Records that have been parsed from the CSV file and validated are shown to the user. Then, as a second request, the user can commit the new records such that they are actually added to a DataSource.

We'll assume this error is happening during that second phase (the commit), but please confirm.

First, you likely have a longstanding usage error here: you pass a DSRequest to startImport(), then return. It's not valid to hold onto a DSRequest, HTTPRequest, or any similar object that is involved in the HTTP lifecycle after the HTTP request has ended. You can hold onto simple data from the original request (like Maps) but you cannot just hold onto HttpServletRequests or any similar request-scoped objects. So you should review the code here to make sure that the service you're starting isn't hanging onto objects from the HTTP lifecycle (it likely is).

Then, you have a second (probably also longstanding) usage error: you have a "serialize the world" problem: JSTranslater serializes POJOs via Java Reflection, discovering accessor methods like "getName()" and calling those. Somewhere on your "Job" object you've got accessors that, possibly indirectly, provide access to some servlet-related object, possibly the HttpServletRequest. That's why you see this:

WARN com.isomorphic.js.JSTranslater:227 - - org.eclipse.jetty.server.Server contains a (potentially indirect) looping reference to itself. Returning null for recursed value.

It's because the serialization system ultimately tries to serialize the entire servlet engine. This, by the way, happens with any Java Reflection-based serializer - it's a well-known problem affecting all such systems, and you can Google around terms like "unintended object graph serialization" and similar terms to see how to solve it. Also, our docs here give you SmartClient-specific ways to solve it:

https://smartclient.com/smartgwtee-r...ava.io.Writer-

Note, in particular, we know of no reason why you would send back this "job" object - unless your client-side code is doing something with it, you could just send back a simple Map.

Finally, the particular error that results here is strange, because it's normally an error that is thrown when you try to write an HTTP response when you aren't done reading the HTTP request. It's strange too, because what's happening is that somewhere in the monstrous object graph you're trying to serialize, there's a Reader object, and reading from it makes Jetty freak out.

However this is happening, given that your code attempts to serialize the entire server object graph while simultaneously breaking the servlet threading model, we suspect that correcting the issues above will also get rid of the thrown exception.

Also note, it's lucky that this code now leads to a straightforward exception. If this was "working" before, it was most likely soaking the CPU and memory of the server to serialize the entire server object graph, then sending enormous unnecessary responses to the client, and also intermittently crashing due to threading issues.

[aderosso]

Yes to confirm, this error is from the second phase, committing.

I agree, passing the DSRequest is not correct, working on a fix for that. I commented out the async code for testing and still get the same result:

    public DSResponse executeAdd(final DSRequest req) throws Exception {
        final int size = req.getRPCManager().getRequests().size();

        final long jobId = getJobSQLService().add(OpeningConstants.IMPORT_OPENINGS, BatchJobStatusConstants.IN_PROGRESS);
        final Map<String, Object> job = new HashMap<>();
        job.put(DSConst.JobSQL.JOB_ID, jobId);

        final DSResponse res = new DSResponse(SmartGWTStatusCode.STATUS_SUCCESS);
        res.setData(job);

        /* Skipping all other request of queue. Process first request from queue and extract all the request from RPCManager. */
        req.getRPCManager().skipRemainingQueue();

        // getImportOpeningService().startImport(req, jobId);

        return res;
    }

The "job" object is simply a map from a SQLDatasource response. We have a job table to store asynchronous work in. The client polls this datasource to get the status while the async process runs. To simplify, I have put only 1 value in the response job object, still get the same result. I've attached a screenshot of the value in the debugger right before returning the DSResponse.

The code above works just fine on 13.0-p20230212, but breaks with 13.0-p20230927. I don't see anything else that I would be serializing, it's just a single long value in a map.

[aderosso]

Also to note, while testing on 13.0-p20230927, I commented out the line

req.getRPCManager().skipRemainingQueue()

and the error goes away. Has the obvious side effect of processing each record 1 by 1, but that would be expected.

[Isomorphic]

Something doesn't add up in what we're being told here..

When you say "the error goes away", are you talking about the stack trace showing runaway serialization, and this warning:

WARN com.isomorphic.js.JSTranslater:227 - - org.eclipse.jetty.server.Server contains a (potentially indirect) looping reference to itself. Returning null for recursed value.

Because those are both definitive signs of runaway serialization and should not be influenced by a call to skipRemainingQueue().

Or are you saying you still see those errors, but somehow, the request completes anyway?

[aderosso]

Sorry, should have been more clear. The error that we're seeing gets back to the client with a status code -90 "TRANSPORT_ERROR", so not operable at all on the latest versions we've tried.

Everything works as expected, no errors, no warnings at all on 13.0-p20230212.

Version 13.0-p20230927 doesn't work at all, the request fails with -90 status, give us the error stack trace and the warnings right before that. However, using version 13.0-p20230927 it does work simply by not calling the skipRemainingQueue().

[Isomorphic]

So specifically, you are saying that with 13.0-p20230212, you do not see any warnings similar to this one:

WARN com.isomorphic.js.JSTranslater:227 - - org.eclipse.jetty.server.Server contains a (potentially indirect) looping reference to itself. Returning null for recursed value.

.. even if you call skipRemainingQueue()?

[aderosso]

Yes correct, with 13.0-p20230212 no warnings or errors at all.

[Isomorphic]

OK, we have been able to reproduce this. The cause is not yet clear, but we'll likely have it resolved by tomorrow.

[Isomorphic]

Thank you for the issue report. We have now identified and fixed the problem. The fix will be present in 13.0p builds as of tomorrow, 12 October (please ensure that the build you test with is dated 2023-10-12 or later)

Regards,
Isomorphic Software Support

[aderosso]

Great, thank you. I will try that out as soon as the build is available.

[aderosso]

Worked perfectly, much appreciated!