Gmail.msgGet gets called without msgId

[ioanmo226]

Tests sometimes timeout with below error

Sample: https://flowcrypt.semaphoreci.com/jobs/11f93653-f3fb-487c-bb27-89df408c2681

....
google-mock listening on 4589100:08
google-mock listening on 4089700:08
got contacts size 101:19
url:GET:/gmail/v1/users/me/messages/?format=metadata /home/semaphore/git/flowcrypt-browser/test/source/mock/lib/mock-util.ts:1204:31
export const parseResourceId = (url: string) => url.match(/\/([a-zA-Z0-9\-_]+)(\?|$)/)![1];04:31
                                                                                       ^04:31
TypeError: Cannot read properties of null (reading '1')04:31
    at parseResourceId (/home/semaphore/git/flowcrypt-browser/test/source/mock/lib/mock-util.ts:12:88)04:31
    at /gmail/v1/users/me/messages/? (/home/semaphore/git/flowcrypt-browser/test/source/mock/google/google-endpoints.ts:224:33)04:31
    at LoggedApi.Api.handleReq (/home/semaphore/git/flowcrypt-browser/test/source/mock/lib/api.ts:130:39)04:31
    at runMicrotasks (<anonymous>)04:31
    at processTicksAndRejections (node:internal/process/task_queues:96:5)04:31
    at <async> '/gmail/v1/users/me/messages/?' (/test/source/mock/google/google-endpoints.ts:220:35)04:31
    at <async> handleReq (/test/source/mock/lib/api.ts:121:24)

[rrrooommmaaa]

TypeError: Cannot read properties of null (reading '1')04:31

Not sure what the reason for this is, possibly some bug, but regarding timeout -- what I see is compose - saving and rendering a draft with RTL text (rich text) fails as Saved indication doesn't appear, I filed issue #5052 and proposed a fix.

[rrrooommmaaa]

Let me have a look at this particular issue

[rrrooommmaaa]

This url /gmail/v1/users/me/messages/?format=metadata is missing a msgId:
Should be: /gmail/v1/users/me/messages/${msgId}?format=metadata

[rrrooommmaaa]

@sosnovsky Can we find out which test caused this hook to fail?
Perhaps, since the port number is dedicated to a test during its execution, we can keep a port->test_name mapping, and dump the necessary info to the logs if not already?

[rrrooommmaaa]

or we should catch it and re-throw HttpClientErr

[sosnovsky]

This url /gmail/v1/users/me/messages/?format=metadata is missing a msgId:
Should be: /gmail/v1/users/me/messages/${msgId}?format=metadata

Real Gmail API returns list of messages when msgId isn't specified (I tested https://www.googleapis.com/gmail/v1/users/me/messages/?format=metadata url), so we should update mock API to have the same behavior.

@sosnovsky Can we find out which test caused this hook to fail?
Perhaps, since the port number is dedicated to a test during its execution, we can keep a port->test_name mapping, and dump the necessary info to the logs if not already?

Probably we can use debug info from semaphore for checking which test caused this fail - debug file

But for this case I think we should just update Gmail mock API to not throw error when there is no msgId in URL.

[sosnovsky]

@rrrooommmaaa have you worked on this task? As it seems tests currently failing mostly because of this issue

[rrrooommmaaa]

Real Gmail API returns list of messages when msgId isn't specified (I tested https://www.googleapis.com/gmail/v1/users/me/messages/?format=metadata url), so we should update mock API to have the same behavior.

As far as I know, the extension never deliberately tries to get a list of messages with this call, so this must be a bug. We can make a temporary support of this operation as part of PR #5053, and leave this issue to be researched and fixed separately. The aim of the mock API is not to mimick Gmail API in full, but to warn us about suspicious calls.

[sosnovsky]

As far as I know, the extension never deliberately tries to get a list of messages with this call, so this must be a bug.

Yeah, I think so, but haven't noticed any code changes which can lead to this call.

We can make a temporary support of this operation as part of PR #5053, and leave this issue to be researched and fixed separately.

I'm not 100% sure that this fix will make tests work, it's just my assumption, but we should try to implement it or another temporary solution to make tests pass.
Later we should also improve Semaphore test reporting for easier detection which tests caused such timeouts.

[rrrooommmaaa]

Later we should also improve Semaphore test reporting for easier detection which tests caused such timeouts.

This particular bug isn't causing timeouts, is it? Timeout failure is supposed to be fixed with #5053

[sosnovsky]

As I remember tests were still failing in #5053, but let's see if they'll pass with your latest commit there

[sosnovsky]

Hi @ioanmo226, I didn't notice such test errors recently, looks like it was fixed and we can close this one?