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

sendState should behave asynchronously #134

Open
cahirodoherty-learningpool opened this issue Oct 25, 2024 · 10 comments · May be fixed by #135
Open

sendState should behave asynchronously #134

cahirodoherty-learningpool opened this issue Oct 25, 2024 · 10 comments · May be fixed by #135
Assignees
Labels

Comments

@cahirodoherty-learningpool
Copy link
Contributor

cahirodoherty-learningpool commented Oct 25, 2024

Subject of the issue/enhancement/features

We have a custom plugin that forces submit of multiple components in one functional batch
(In essence:

_.each(largeArrayOfComponentViews, (componentView) => {
        componentView.onSubmitClicked();
      });

)

When it comes to handling a large volume of components, we have found that this effectively creates a race condition and the state that comes back may actually not be the last state sent, thus cutting off some question values when the state is loaded again on subsequent views.

I believe setting an await on https://github.com/adaptlearning/adapt-contrib-xapi/blob/master/js/XAPI.js#L1082 should help us out in this regard

@cahirodoherty-learningpool
Copy link
Contributor Author

@oliverfoster Any thoughts on this?

@oliverfoster
Copy link
Member

oliverfoster commented Nov 4, 2024

The state is concurrently read in once and updated multiple times?

How is an old state read in after an update is sent? What is the exact mechanism / order of flow?

Strategically, it's best usually to add a "last updated" timestamp on the state. Such that only states that are newer than the local/previous are accepted as updates and retries are attempted on conflict.

@cahirodoherty-learningpool
Copy link
Contributor Author

cahirodoherty-learningpool commented Nov 7, 2024

Basically its a one-button submit for all components:

onSubmit() {
    _.each(this.componentViews, (componentView) => {
        componentView.onSubmitClicked();
    });
}

@oliverfoster
Copy link
Member

Yes, I don't understand where a state is being read in and in conflict. I see that submitting multiple questions at the same time would rapidly send statements.

@cahirodoherty-learningpool
Copy link
Contributor Author

So based on examining the attached LRS, the component statements are registered with completion/correctness etc.
The problem lies with the overall state held in the LRS. So when the course is re-opened, the LRS state is loaded and many components appear to not have been completed. My hunch is that the large number of POST requests to update the course state are somehow being processed in the incorrect order and thus the last state to be saved, is not necessarily the last state intended to be processed.
From your comment about 'last updated', is this something that is in place by default on our submissions? I can't locate where that would be getting done

@oliverfoster
Copy link
Member

I see, keeping a last updated date won't help you in this situation then, that would be added to the state when the client and sever need to be synched so that you can check which state is newer when handling copies.

A debounce for sending the lrs state would probably help here. Something like a half a second debounce, such that multiple updates can happen in quick succession in the client side and then a single update is sent to the server after the period has elapsed?

@cahirodoherty-learningpool
Copy link
Contributor Author

Hi Oliver, yeah thats what we put in place to see us through and reports from the client seem to be positive. If thats the recommended solution and we aren't going to take any further action as a community then I'm happy to close this issue off now

@oliverfoster
Copy link
Member

Could you push the debounce to the public repo if you haven't already?

@cahirodoherty-learningpool
Copy link
Contributor Author

Its on the custom plugin side wrapped around the onSubmitClicked calls that I mentioned above. Were you talking about placing something in xAPI extension?

@oliverfoster
Copy link
Member

oliverfoster commented Dec 2, 2024

Here seems like a good place:

this.listenTo(Adapt, 'state:change', this.sendState);

If you put the following in the preinitialize function, it should just work, maybe/maybe not:

this.sendState = _.debounce(this.sendState.bind(this), 500);

Or move this into a new debounced function using a similar method:

adapt-contrib-xapi/js/XAPI.js

Lines 1082 to 1088 in 121b55b

this.xapiWrapper.sendState(activityId, actor, collectionName, registration, newState, null, null, (error, xhr) => {
if (error) {
Adapt.trigger('xapi:lrs:sendState:error', error);
}
Adapt.trigger('xapi:lrs:sendState:success', newState);
});

Or you'll have to make a queue for states to be sent for the relevant collectionName perhaps... I'm not quite sure.

@swashbuck swashbuck added the bug label Dec 4, 2024
@cahirodoherty-learningpool cahirodoherty-learningpool moved this from Assigned to Needs Reviewing in adapt_framework: The TODO Board Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs Reviewing
Development

Successfully merging a pull request may close this issue.

3 participants