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

Refactoring #244 - Comments robustness #250

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ function writeThreadcapChunk(processedNodeId, threadcap, sentCommenters, res) {
threadcapChunk.commenters[comment.attributedTo] = threadcap.commenters[comment.attributedTo];
}

res.write(JSON.stringify(threadcapChunk))
res.write(JSON.stringify(threadcapChunk) + '\n')
}

// ------------------------------------------------
Expand Down
56 changes: 45 additions & 11 deletions ui/src/components/Comments/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,18 +129,50 @@ export default class Comments extends React.PureComponent<IProps, IState> {

const thisComponent = this;

let incompletePartialResponse: string = '';

await reader.read().then(function processChunk({done, value}) {
/**
* if everything goes right, value should be singe complete JSON encoded object,
* but we have seen that a reverse proxy can mess this up.
* Thus, we introduced '\n' as a delimiter, and we assume that value could be also be
* fragments of one or more concatenated JSON objects.
* e.g.:
* '{"root":' - one incomplete JSON object
* '{...}\n' - a complete JSON object (indicated by ending in \n)
* '{...}\n{"root":' - complete JSON object followed by an incomplete JSON object
* '}\n{"root":' - an final fragment of a JSON object followed by an incomplete JSON object
*/
if(done) {
thisComponent.setState({
loadingComments: false
});
return;
}
const parsedChunk = JSON.parse(new TextDecoder().decode(value));

updateResponseBody(responseBody, parsedChunk);

const decodedValue = new TextDecoder().decode(value);
let partialResponses = decodedValue.split('\n');

if(partialResponses.length > 0) {
partialResponses[0] = incompletePartialResponse + partialResponses[0];

incompletePartialResponse = partialResponses[partialResponses.length-1];

partialResponses = partialResponses.slice(0, -1);

// normally, at this point partialResponses.length equals 1, but
// if multiple chunks were buffered and delivered as one, it could be
// > 1

if(incompletePartialResponse || partialResponses.length > 1) {
console.warn('Comments: Unexpected chunked responses from the comments API - a workaround is in place to ensure functional correctness, but performance is impacted.')
}
}

const parsedPartialResponses = partialResponses.map((partialResponse) => JSON.parse(partialResponse));

updateResponseBody(responseBody, parsedPartialResponses);

const stateToSet: any = {
showComments: true,
};
Expand All @@ -151,14 +183,16 @@ export default class Comments extends React.PureComponent<IProps, IState> {
return reader.read().then(processChunk);
});

function updateResponseBody(responseBody, parsedChunk) {
responseBody.roots = responseBody.roots.concat(parsedChunk.roots);
for(let key in parsedChunk.nodes) {
responseBody.nodes[key] = parsedChunk.nodes[key];
}
for(let key in parsedChunk.commenters) {
responseBody.commenters[key] = parsedChunk.commenters[key];
}
function updateResponseBody(responseBody, parsedPartialResponses: Array<any>) {
parsedPartialResponses.forEach((parsedPartialResponse) => {
responseBody.roots = responseBody.roots.concat(parsedPartialResponse.roots);
for(let key in parsedPartialResponse.nodes) {
responseBody.nodes[key] = parsedPartialResponse.nodes[key];
}
for(let key in parsedPartialResponse.commenters) {
responseBody.commenters[key] = parsedPartialResponse.commenters[key];
}
})
}
}
else {
Expand Down