Skip to content

Promises instead callbacks. Now server not crashed if send bad token. #3

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.

Sign up for GitHub

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

Closed
OsirisAnubiz wants to merge 2 commits into from
Closed

Promises instead callbacks. Now server not crashed if send bad token. #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f67d00
Use promises instead callbacks. Now server not crashed if send invalid
OsirisAnubiz Mar 22, 2023
20c165c
Update session.js
OsirisAnubiz Mar 22, 2023
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
20 changes: 8 additions & 12 deletions JavaScript/session.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,17 @@ class Session extends Map {
return session;
}

static restore(client) {
static async restore(client) {
const { cookie } = client;
if (!cookie) return;
const sessionToken = cookie.token;
if (sessionToken) {
return new Promise((resolve, reject) => {
storage.get(sessionToken, (err, session) => {
if (err) reject(new Error('No session'));
Object.setPrototypeOf(session, Session.prototype);
client.token = sessionToken;
client.session = session;
resolve(session);
});
});
}
if (!sessionToken) return;
const session = await storage.get(sessionToken);
if (!session) return;
Object.setPrototypeOf(session, Session.prototype);
client.token = sessionToken;
client.session = session;
return session;
}

static delete(client) {
Expand Down
56 changes: 22 additions & 34 deletions JavaScript/storage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,63 +1,51 @@
'use strict';

const fs = require('node:fs');
const fs = require('node:fs').promises;
const path = require('node:path');
const v8 = require('node:v8');

const PATH = `${__dirname}/sessions`;

const safePath = (fn) => (token, ...args) => {
const callback = args[args.length - 1];
if (typeof token !== 'string') {
callback(new Error('Invalid session token'));
return;
}
const safePath = (promise) => (token, ...args) => {
Copy link
Copy Markdown
Member

@tshemsedinov tshemsedinov Mar 22, 2023

Choose a reason for hiding this comment

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

promise is not an instance of Promise, it's an AsyncFunction so naming is confusing

if (typeof token !== 'string') throw new Error('Invalid session token');
const fileName = path.join(PATH, token);
if (!fileName.startsWith(PATH)) {
callback(new Error('Invalid session token'));
return;
}
fn(fileName, ...args);
if (!fileName.startsWith(PATH)) throw new Error('Invalid session token');
return promise(fileName, ...args)
};

const readSession = safePath(fs.readFile);
const writeSession = safePath(fs.writeFile);
const deleteSession = safePath(fs.unlink);

class Storage extends Map {
get(key, callback) {
async get(key) {
const value = super.get(key);
if (value) {
callback(null, value);
return;
if (value) return value;
let data;
try {
data = await readSession(key);
} catch (err) {
Copy link
Copy Markdown
Member

@tshemsedinov tshemsedinov Mar 22, 2023

Choose a reason for hiding this comment

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

err defined but not used

Copy link
Copy Markdown
Author

@OsirisAnubiz OsirisAnubiz Mar 23, 2023

Choose a reason for hiding this comment

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

Would it be better to create 2 pull requests? First for fix crashing server. Second for async/await syntax.

Copy link
Copy Markdown
Member

@tshemsedinov tshemsedinov Mar 23, 2023

Choose a reason for hiding this comment

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

It would be better to open separate PRs from different branches

Copy link
Copy Markdown
Author

@OsirisAnubiz OsirisAnubiz Mar 23, 2023

Choose a reason for hiding this comment

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

Thank you. I will do as you said. I apologize for the inconvenience.

return undefined;
}
readSession(key, (err, data) => {
if (err) {
callback(err);
return;
}
console.log(`Session loaded: ${key}`);
const session = v8.deserialize(data);
super.set(key, session);
callback(null, session);
});
console.log('Session loaded: ${key}');
const session = v8.deserialize(data);
super.set(key, session);
return session;
}

save(key) {
async save(key) {
const value = super.get(key);
if (value) {
const data = v8.serialize(value);
writeSession(key, data, () => {
console.log(`Session saved: ${key}`);
});
await writeSession(key, data);
console.log(`Session saved: ${key}`);
}
}

delete(key) {
async delete(key) {
console.log('Delete: ', key);
deleteSession(key, () => {
console.log(`Session deleted: ${key}`);
});
await deleteSession(key);
console.log(`Session deleted: ${key}`);
}
}

Expand Down