Changes for fallback key test

[MidhunSureshR]

Contains code to return the room-id from create-room action.

[michaelkaye]

Just to check my understanding here:

We're using a promise and making the code async not to become particularly async ourselves but to manage that the function createRoom needs information that comes from a callback (line 37) rather than something it can do directly?

Did this work well / was it easy? I'm wondering if i should refactor away the use of jquery in favor of this sort of style.

[michaelkaye]

The error handling i think needs to be addressed; though would also be happy with a note on how it's actually safe, if i've misunderstood something.

[michaelkaye]

Do we mean to change JSON value -> Record here?

[MidhunSureshR]

JSONValue is a type we define in trafficlight.spec.ts and it's not exported. Record here is basically saying that this function returns some object. Ideally we'd define the type explicitly, for eg:

type TimelineContent = { 
    user: string[]; 
    e2e_issues: string[];
}    

[michaelkaye]

What happens to the flow if we don't send a result? We need to handle that as an error case (ActionRecord can be of type undefined) otherwise I think we'll end up in a loop doing a poll then trying to perform the action again

[michaelkaye]

Maybe we should move the whole set of methods to a promise interface and stop returning undefined; instead making the promise error. IDK.

[MidhunSureshR]

Only certain actions like idle and exit return undefined. All other actions will return something (most likely a string).
If these other actions don't return something that means an error was thrown while they were being executed. We have hooked to the cypress events so that these are caught and reported to the server. See

trafficlight-adapter-element-web/cypress/e2e/trafficlight/trafficlight.spec.ts

Lines 56 to 87 in 51d7e79

	Cypress.on('uncaught:exception', (e, runnable) => {
	console.log("uncaught exception", e.message);
	const errorUrl = `${Cypress.env('TRAFFICLIGHT_URL') }/client/${ Cypress.env('TRAFFICLIGHT_UUID') }/error`;
	const errorPath = e.stack?.split("\n")[0];
	const body = JSON.stringify({
	error: {
	type: e.name,
	details: e.message,
	path: errorPath,
	},
	});
	fetch(errorUrl, { method: "POST", body });
	return false;
	});
	Cypress.on('fail', (e) => {
	const errorUrl = `${Cypress.env('TRAFFICLIGHT_URL') }/client/${ Cypress.env('TRAFFICLIGHT_UUID') }/error`;
	const errorPath = e.stack?.split("\n").slice(1).join("\n");
	const body = JSON.stringify({
	error: {
	type: e.name,
	details: e.message,
	path: errorPath,
	},
	});
	fetch(errorUrl, {
	method: "post",
	body,
	headers: { 'Content-Type': 'application/json' },
	});
	throw e;
	});

[MidhunSureshR]

I agree that this can probably be written in a better way but I suggest that we just create an issue for now (since this code was not introduced by the changes in this PR).

[MidhunSureshR]

Just to check my understanding here:

We're using a promise and making the code async not to become particularly async ourselves but to manage that the function createRoom needs information that comes from a callback (line 37) rather than something it can do directly?

Did this work well / was it easy? I'm wondering if i should refactor away the use of jquery in favor of this sort of style.

Cypress doesn't use JS promises, they use their own promise like objects. See https://docs.cypress.io/guides/core-concepts/introduction-to-cypress#Commands-Are-Asynchronous.
Also using promise and cypress together is very finicky and as such this was the only way I could get it to work. I didn't really think about using Cypress.$ to do this but I'm not sure if that would change anything.
I want to wait for getRoomIdFromName to return the id before I return from createRoom.