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

Firestore adapter cannot write conversation sessions #118

Closed
sartoshi-foot-dao opened this issue Aug 24, 2022 · 6 comments
Closed

Firestore adapter cannot write conversation sessions #118

sartoshi-foot-dao opened this issue Aug 24, 2022 · 6 comments

Comments

@sartoshi-foot-dao
Copy link

Problem

Firestore cannot store conversations session data with current adapter.

Cause

Firestore cannot encode the function definitions of the stored update objects under log e.g. copy, forward.

While the Admin SDK throws the error Error: Cannot encode value: (chat_id, other, signal), which wasn't too informative, testing with the Web SDK produced a more useful error Function setDoc() called with invalid data. Unsupported field value: a function.

Solution (?)

Perhaps the Firestore adapter should JSON.stringify / parse when writing to and reading from the database, at least for conversation sessions? It's slightly inelegant but can get the job done.

I could well be missing something very obvious, so would appreciate somebody suggesting a better approach. Thanks!

@KnorpelSenf
Copy link
Member

Which version of the plugin are you using?

@sartoshi-foot-dao
Copy link
Author

"grammy": "1.9.2"
@grammyjs/conversations": "^1.0.2"

Sorry, I was a little presumptuous in my original post as I am not using the officially supported Firestore adapter to save on referencing a keyfile. As my bot is hosted on GCP Cloud Functions, I use getFirestore from firebase-admin/firestore, which accesses the same service as Firestore from @google-cloud/firestore.

I adapted the official adapter to suit my approach. Here it is:

const firebaseAdapter: StorageAdapter<SessionData> = {
	read: async (key: string) => {
		const snapshot = await db.collection('sessions').doc(key).get();
		return snapshot.data() as SessionData | undefined;
	},
	write: async (key: string, value: SessionData) => {
		console.log(util.inspect(value, false, null, true /* enable colors */));
		await db
			.collection('sessions')
			.doc(key)
			.set(value);
	},
	delete: async (key: string) => {
		await db.collection('sessions').doc(key).delete();
	}
};

Which I then use as follows:

bot.use(
			session({
				initial() {
					return {};
				},
				storage: firebaseAdapter
			})
		);

I shall be more precise with future issues. Apologies for any inconveniences caused and thank you for your tireless work on this project!

@sartoshi-foot-dao
Copy link
Author

sartoshi-foot-dao commented Aug 25, 2022

To add as a note:

Seems like JSON.stringify() removes any function definitions found within the JSON. I wonder if some kind of preprocess could lint the function definitions from the conversation session data, unless they are particularly necessary for conversation replay?

Edit:
Also seems that the functions that were removed by JSON.stringify() are reintroduced to the new context object upon conversation replay.

Having my custom adapter stringify and parse works; it's just not as it-just-works as it could be...any suggestions?

@sartoshi-foot-dao
Copy link
Author

Found a workaround without modifying official Firestore adapter with JSON.stringify.

Note: update dependencies to latest versions of grammy and @grammyjs/conversations.

// no need for Firestore config if using Firebase with Cloud Functions
const db = getFirestore();

// should silence Error: Value for argument is not a valid Firestore document. Cannot use "undefined" as a Firestore value
db.settings({
	ignoreUndefinedProperties: true
});

Problem solved.

I recognise that the guide for Hosting on GCP is 'coming soon'. If a ancillary note on Firebase Functions would be welcome, I'd be happy to organise my learnings and document pitfalls. Any contribution guidance would be appreciated too @KnorpelSenf

@KnorpelSenf
Copy link
Member

KnorpelSenf commented Aug 25, 2022

Great to see this solved!

@Satont this looks like a good solution. I remember doing this myself in a different project. It would be interesting to know why Google didn't set this option by default. Could it be mentioned in the respective README file?

@sartoshi-foot-dao you're more than welcome to write this guide. I was initially planning on writing a guide for Google Cloud Functions not Firebase functions, but I guess they're using the same service internally anyway so either solution is fine for me.

Regarding website contributions, I can recommend our contribution guide: https://github.com/grammyjs/website/blob/main/CONTRIBUTING.md

Feel free to ping me or the website maintainer @roj1512 either on GitHub or on Telegram (https://t.me/grammyjs) if anything else is unclear :)

@sartoshi-foot-dao
Copy link
Author

Thank you! I'll give it a shot tomorrow afternoon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants