linagora / tmail-backend

GNU Affero General Public License v3.0
31 stars 17 forks source link

CalendarEvent reply extension throw serverFail error #1006

Closed quantranhong1999 closed 2 months ago

quantranhong1999 commented 3 months ago

Description of the bug

on tmail.linagora.com: Given @tddang-linagora invited to an event He replies using e.g CalendarEvent/accept via Postman Then TMail backend returns serverFail error with a null description.

Reproduction Steps

Can try to reproduce this on prod as well as staging.

Expected result

TMail does not return such serverFail error.

Context

tmail.linagora.com tmail-backend 0.9.0

Additional information

ics file: invite.zip

quantranhong1999 commented 3 months ago

error log on the backend side:

{
    "timestamp": "2024-04-10T04:26:05.637Z",
    "level": "ERROR",
    "thread": "parallel-7",
    "mdc": {
        "method": "CalendarEvent/maybe",
        "protocol": "JMAP",
        "username": "tddang@linagora.com"
    },
    "logger": "org.apache.james.jmap.method.Method",
    "message": "Failed executing a JMAP method",
    "context": "default",
    "exception": "java.lang.IllegalArgumentException: Can not reply when not invited to attend\n\tat com.google.common.base.Preconditions.checkArgument(Preconditions.java:143)\n\tat com.linagora.tmail.james.jmap.model.CalendarEventReplyGenerator$.generate(CalendarEventReplyGenerator.scala:32)\n\tat com.linagora.tmail.james.jmap.method.CalendarEventMailReplyGenerator.$anonfun$generateAttachmentPart$1(CalendarEventReplyPerformer.scala:174)\n\tat reactor.core.publisher.MonoCallable$MonoCallableSubscription.request(MonoCallable.java:137)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171)\n\tat reactor.core.publisher.MonoFlatMap$FlatMapMain.request(MonoFlatMap.java:194)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171)\n\tat reactor.core.publisher.MonoFlatMap$FlatMapInner.onSubscribe(MonoFlatMap.java:291)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:96)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:96)\n\tat reactor.core.publisher.MonoFlatMap$FlatMapMain.onSubscribe(MonoFlatMap.java:117)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:96)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:96)\n\tat reactor.core.publisher.MonoCallable.subscribe(MonoCallable.java:48)\n\tat reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76)\n\tat reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:165)\n\tat reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:158)\n\tat reactor.core.publisher.FluxSwitchIfEmpty$SwitchIfEmptySubscriber.onNext(FluxSwitchIfEmpty.java:74)\n\tat reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)\n\tat reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:129)\n\tat reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:129)\n\tat reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:82)\n\tat reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:129)\n\tat reactor.core.publisher.FluxPublishOn$PublishOnSubscriber.runAsync(FluxPublishOn.java:440)\n\tat reactor.core.publisher.FluxPublishOn$PublishOnSubscriber.run(FluxPublishOn.java:527)\n\tat reactor.core.scheduler.WorkerTask.call(WorkerTask.java:84)\n\tat reactor.core.scheduler.WorkerTask.call(WorkerTask.java:37)\n\tat java.base/java.util.concurrent.FutureTask.run(Unknown Source)\n\tat java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(Unknown Source)\n\tat java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)\n\tat java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)\n\tat java.base/java.lang.Thread.run(Unknown Source)\n"
}

I suspect we have a bug when retrieving the username and checking if the user is in the ATTENDEE list (while it should not be the case as the user here is actually invited to the event).

BTW upon Can not reply when not invited to attend exception, we should catch and not throw serverFail.

quantranhong1999 commented 3 months ago

cc @Arsnael can our team prioritize this a bit more? It blocks the mobile team for now.

Arsnael commented 3 months ago

I think I never said you could not tackle this issue, please take care of it :)

quantranhong1999 commented 3 months ago

I think I never said you could not tackle this issue, please take care of it :)

Yes, sorry but I am busy with the quorum queue topic :(

chibenwa commented 3 months ago

So I reproduced the issue using the provided ICS into CalendarEventReplyGeneratorTest

I can confirm the issue is linked to unhandled folding:

ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=tddang@linagora.com;X-NUM-GUESTS=0:mailto:tddang@linagora.com

(ICAL4J filters the properties out)

From what I tested, the ICAL4J supports compatibility hints solving this:

CompatibilityHints.setHintEnabled(CompatibilityHints.KEY_RELAXED_UNFOLDING, true);
chibenwa commented 3 months ago

Won't have the time to continue on it, so please include it on the next kanban.

Arsnael commented 3 months ago

Won't have the time to continue on it, so please include it on the next kanban.

I think @vttranlina is on it already :)

vttranlina commented 3 months ago

pr https://github.com/linagora/tmail-backend/pull/1011

Arsnael commented 3 months ago

Fix deployed on staging

quantranhong1999 commented 2 months ago

I reopened the issue as serverFail error still happens on preprod (reported by @tddang-linagora).

Image: linagora/tmail-backend:distributed-0.10.0-rc1

CalendarEvent/reject for example:

{
    "sessionState": "2c9f1b12-b35a-43e6-9af2-0106fb53a943",
    "methodResponses": [
        [
            "error",
            {
                "type": "serverFail",
                "description": "/root/eml-template/calendar_reply_declined-en.eml (No such file or directory)"
            },
            "c0"
        ]
    ]
}

Actually, the eml template files were not in the pods:

root@tmail-jmap-67ff4db9bf-9d7h8:~# cat /root/eml-template/calendar_reply_declined-en.eml
cat: /root/eml-template/calendar_reply_declined-en.eml: No such file or directory

Weird as we tried to mount it via JIB though.

vttranlina commented 2 months ago

latest master branch eml template already exists image

Arsnael commented 2 months ago

I don't see it in distributed 0.10.0 running on staging though, the folder is missing (thus the error)

quantranhong1999 commented 2 months ago

latest master branch eml template already exists

Are you testing with your locally built image?

I tested with images on the docker hub and there is not: image

vttranlina commented 2 months ago

Are you testing with your locally built image?

yes

quantranhong1999 commented 2 months ago

yes

OK, likely some dark magic the the CI build

Arsnael commented 2 months ago

OK, likely some dark magic the the CI build

Not necessary, I see a difference of commits between master and the tag, even if it seems unrelated, let me have a look at it first

quantranhong1999 commented 2 months ago

should fix: https://github.com/linagora/tmail-backend/pull/1018

Arsnael commented 2 months ago

should fix: #1018

Can you do the same for the pg-app on postgresql branch please? :)

quantranhong1999 commented 2 months ago

Can you do the same for the pg-app on postgresql branch please? :)

Postgres app should be good (no dedicated ci profile for jib).

quantranhong1999 commented 2 months ago

I tested and preprod should be fine so far.

image

image

Arsnael commented 2 months ago

Will make a new release cycle including this, thanks