Occasional crash without correct error handling while using Magister.ready(callback)

[gruijter]

The function Magister.ready(callback) crashes for unknown reasons. This happens occasionally. Below function is called every 10 minutes, with the exact same credentials, and crashes after an irregular number of calls (usually within 10 calls). see response at the bottom.

function getCourse(credentials, callback) {
    new Magister.Magister(credentials).ready(function() {
        this.currentCourse(function(error, result) {
            if (error) {
                console.log("got error: " + error);
                callback(error, null);
            } else {
                var courseInfo = {
                    period: result.schoolPeriod(), // e.g. 1516
                    begin: result.begin(),         // e.g. Sat Aug 01 2015 00:00:00 GMT+0200 (West-Europa (zomertijd))
                    end: result.end(),             // e.g. Sun Jul 31 2016 00:00:00 GMT+0200 (West-Europa (zomertijd))
                    type: result.type(),           // e.g. { id: 1346, description: '1 gymnasium' },
                    group: result.group(),         // e.g. { id: 6150, description: '1gb', locationId: 0 },
                    classesById: {}
                };
                result.classes(function(error, courseClasses) {
                    for (var courseClass of courseClasses) {
                        courseInfo.classesById[courseClass.id()] = {
                            id: courseClass.id(),                     // e.g. 532518
                            abbreviation: courseClass.abbreviation(), // e.g. 'ne'
                            description: courseClass.description(),   // e.g. 'Nederlandse taal'
                            number: courseClass.number()              // e.g. 1
                        };
                    };
                    callback(null, courseInfo);
                });
            };
        });
    });
}

Error: Not done with logging in or errored during logging in! (did you use Magister.ready(callback) to be sure that logging in is done?)
    at Magister.root.Magister.Magister._forceReady (/node_modules/magister.js/lib/node/magister-node.js:3327:15)
    at Magister.root.Magister.Magister.currentCourse (/node_modules/magister.js/lib/node/magister-node.js:2745:12)
    at Magister.<anonymous> (/drivers/student/driver.js:352:9)
    at wrapper (/node_modules/magister.js/node_modules/lodash/index.js:3095:19)
    at Magister.root.Magister.Magister._setErrored (/node_modules/magister.js/lib/node/magister-node.js:3363:9)
    at /node_modules/magister.js/lib/node/magister-node.js:3452:30
    at Request._callback (/node_modules/magister.js/lib/node/magister-node.js:5379:16)
    at Request.self.callback (/node_modules/magister.js/node_modules/request/request.js:198:22)
    at emitTwo (events.js:87:13)
    at Request.emit (events.js:172:7)

[gruijter]

After doing error handling I found out that occasionally an error is produced that causes the problems:
Ongeldig account of verkeerde combinatie van gebruikersnaam en wachtwoord. Probeer het nog eens of neem contact op met de applicatiebeheerder van de school.

[lieuwex]

Is that error correct?

[gruijter]

The error is occasionally (about once every 5-10 times) presented when using this code:

new Magister.Magister(credentials).ready(function (error) {
    if (error) {
      console.log(error.message);
      callback(error.message, null);
      return;
    }
    else {

Since I'm using the exact same credentials everytime I call the function, I am sure that the credentials are valid. So the "invalid account" or "wrong username / password" error is not correct.

[lieuwex]

Okay that's weird, I will look into it.

[gruijter]

I found out that the error happens when I do a new Magister.ready(callback) before the previous call has fully finished loading data. E.g. loading all grades can take 5 seconds, and in the mean time node.js will already start the next Magister.ready(callback) e.g. to get appointments > the error then happens.

[gruijter]

So I think it has to do with opening multiple sessions to Magister simultaneously

[gruijter]

@lieuwex :

off topic:

do you know if the student id ( profileInfo().id() ) is unique globally? I mean if a student moves to another school, will he keep the same id? Or is the school in charge of "making up" a unique id only within their own school domain?

And I have the same question for the username. (And why is the username not the same as the id?)

[lieuwex]

I found out that the error happens when I do a new Magister.ready(callback) before the previous call has fully finished loading data. E.g. loading all grades can take 5 seconds, and in the mean time node.js will already start the next Magister.ready(callback) e.g. to get appointments > the error then happens.

So I think it has to do with opening multiple sessions to Magister simultaneously

That seems like it, I would not use use setInterval but I would call setTimeout when you got all the results.

do you know if the student id ( profileInfo().id() ) is unique globally

AFAIK the IDs are only unique inside the 'school domain', so two schools could have overlapping IDs. If you want to have unique IDs I advise you to prefix MagisterSchool#id to the id. (accessible within the callback to Magister#ready as this.magisterSchool.id)

And I have the same question for the username. (And why is the username not the same as the id?)

I guess that has to do with the fact that the student ID is always numerical, but the username doesn't have to be. Most of the times it is, but it could contain letters too AFAIK.

[lieuwex]

I will take a look to if I can improve the error when multiple sessions are created to Magister simultaneously

[tkon99]

I guess that has to do with the fact that the student ID is always numerical, but the username doesn't have to be. Most of the times it is, but it could contain letters too AFAIK.

Mine is completely alphabetical: tkonings

[gruijter]

@lieuwex Thx for the answer on the off-topics!

On topic however I think that an improved error or using setTimeout to prevent multiple sessions from occuring is fighting the symptom in stead of fighting the cause of the problem. In my opinion it should always be possible to have multiple simultaneous sessions open. Since this is for Node.JS I never know when an event occurs that will trigger a session. And because of asynchronous execution of the code it is very likely that multiple sessions will be opened at certain times. The use case is as described before:

E.g. loading all grades can take 5 seconds, and in the mean time node.js will already start the next Magister.ready(callback) e.g. to get appointments

[lieuwex]

@gruijter Yeah, I will try to fix it. But I wouldn't open multiple sessions to Magister anyway since it will trigger the ratelimit sooner and there isn't really a reason for it in your usecase.

[gruijter]

@lieuwex : Thx!

and what is the ratelimit? Is there any documentation I can read to learn about these kind of things? I can only find a (not very comprehensive) document at: http://simplyapps.nl/MagisterJS/docs/index.html

[lieuwex]

@gruijter Well, we don't currently handle the ratelimit well (see #38). So it's also not documented, but when you send a lot of requests you get like around 15 seconds of timeout IIRC. I'm still hoping to handle the ratelimit so that we delay requests to the server until the ratelimit is over.

[gruijter]

@lieuwex: any news on handling multiple sessions simultaneously? I am now not able to use the MagisterJS because of the limitation. It causes erratic/unpredictable behavior in the app I wanted to create.

[lieuwex]

@gruijter It seems that logging at the same time is just not possible using Magister, so it isn't really an issue with Magister.js.
We could fix this by sharing state in-between Magister objects, but that isn't really nice for a library to do and we would be doing more stuff than we really aim for.

I recommend doing a workaround in your own code. For example creating a wrapper which makes sure new Magister objects are only created after the old one has been done logging in or by caching Magister instances (also adds a performance bonus by not having to login all the time).

[tomsmeding]

I'm going to second @lieuwex's last sentence. It sounds like you're logging in multiple times with the same credentials, which isn't really how the library is designed. Log in once and cache the Magister object; it will handle any necessary re-logins (because the session might expire for example) by itself, without any intervention from your side. You'll issue far fewer requests, and will have a much lower chance of hitting the ratelimit. And if you do hit it, it's magister that's wanting you to stop, not this library. :)

[gruijter]

@tomsmeding @lieuwex :

ok, thanks for this insight!

Could you give an example code on how to cache the entire magister object? I need to get the profileInfo, the currentCourse (classes and grades) and appointments.

[lieuwex]

Sorry for the delay, but this is an example of caching the magister object, this requires the lru-cache module. If you only want to cache 1 to 5 magister objects a LRU cache isn't really needed and you could just use an object.

var Magister = require('magister.js').Magister;
var LRU = require('lru-cache');

const cache = LRU({
    max: 50,
    maxAge: 1000 * 60 * 60 * 2, // 2 hours
});

var getMagisterObject = function (schoolurl, username, password) {
    var id = schoolurl + '_' + username;
    var magister = cache.get(id);

    if (magister == null) {
        magister = new Magister({
            school: {
                url: schoolurl,
            },
            username: username,
            password: password,
        });
        cache.set(id, magister);
    }

    return magister;
};

// Usage example:

getMagisterObject(
    'https://<schoolname>.magister.net',
    '<username>',
    '<password>'
).ready(function (err) {
    if (err != null) {
        console.error(err);
        return;
    }

    // code here
});

[gruijter]

@lieuwex Thx for this! You mention I could also use an object in stead of a LRU cache. I think this is what I'm looking for. How would this work?

Now I've based my code on the examples from this page: http://simplyapps.nl/MagisterJS/
Every of the three given example actions starts a new Magister.Magister with full login. Now as I understand from you this is not the right way to do it when I want to get multiple information types (almost) simultaneously (e.g. profileInfo, the currentCourse classes and grades, and appointments).

In the documentation @: http://simplyapps.nl/MagisterJS/docs/classes/Magister.html
I come across [sessionId] and [keepLoggedIn=true](which is true by default I noticed). But I have no clue how to use this.

Could you help me out with some example code when I don't want to use LRU?

I appreciate it!

[bwired-nl]

Same for me 😀
Many thanks

[gruijter]

@bwired-nl and all others who are interested: I have published my Magister App for Homey on GitHub:
https://github.com/gruijter/com.gruijter.magister/

[lieuwex]

@gruijter Every example on the homepage can be seen as different files. In a real app you wouldn't log in everytime you want to fetch anything, but just once.

keepLoggedIn is true per default, it makes sure that the session created by logging in never expires. sessionId doesn't have to be used, you can use it if you already have the ID of an older session you want to login to.

I created a pull request at your repo which should fix your problem by caching the object. I didn't test this or anything so I'm not sure if I made a mistake somewhere.

[gruijter]

@lieuwex That's great work! I'll try to see how this works out (btw: you forgot to put var in front of getMagister). Being a complete noob I however don't get what is now exactly being cached. Is it all data belonging to a student? I want to poll Magister every 10 minutes to check if there is a new grade or a change in appointments. Would this caching have an effect on that?

[lieuwex]

@gruijter Woops you're right.

But nope, it's just the session that gets cached, not the data. This has no effect whatsoever apart from no errors and fewer network requests.

[gruijter]

Good news. I now have it implemented in the app and first impression is that it works great. Thx for your help in this.