This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5164-e7e5d4ee74cb7bf891218518c0c287cf3e9abdb4 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 0a42fcdbc20732ada9614bc77a0cfedd05cf6074 Author: Meng Wang <[email protected]> AuthorDate: Sun May 24 03:25:42 2026 -0700 fix: surface SMTP failures from gmail share endpoint to UI (#5164) ### What changes were proposed in this PR? `GmailResource.sendEmailRequest` discarded the `Either[String, Unit]` returned by `sendEmail`. The endpoint itself was typed `Unit`, so JAX-RS responded `204 No Content` regardless of outcome — any SMTP failure (invalid credentials, network error, etc.) was logged server-side but the HTTP layer reported success, and the frontend's `next:` callback fired a green "Email sent successfully" toast even though no mail was delivered. This PR closes that gap in two layers: **Backend (`GmailResource.scala`)** — pattern-match the `Either` and throw `WebApplicationException(error, Response.Status.BAD_GATEWAY)` on `Left`. JAX-RS converts the exception into an HTTP 502, which triggers Angular's `error:` callback on the frontend side. This matches the existing codebase convention in `AdminUserResource` (`Email already exists`, 409) and `UserResource` (`User not found`, 404). **Frontend (`gmail.service.ts`)** — the existing `error:` branch only `console.error`'d. Add a `notificationService.error(...)` call mirroring the existing `notifyUnauthorizedLogin` error handler in the same file (lines 56–61). The user-facing toast uses a generic message ("Failed to send email. Please try again or contact admin.") rather than the raw SMTP error, to avoid exposing protocol-level details like `535-5.7.8 Username and Password not accepted` to end users; the specific SMTP error continues to land in the backend log and the browser console for debugging. ### Any related issues, documentation, discussions? Closes #5161. ### How was this PR tested? Added `gmail.service.spec.ts` (Vitest + `HttpClientTestingModule`) covering both contracts: - HTTP 2xx → `notificationService.success("Email sent successfully")` fires - HTTP 502 → `notificationService.error("Failed to send email. ...")` fires No backend test added: `GmailResource` has no JAX-RS controller test harness, and building one would require refactoring `sendEmail` out of the companion object into an injectable dependency — disproportionate to a five-line fix. Verified locally: - `sbt "WorkflowExecutionService/scalafmtCheck" "WorkflowExecutionService/compile"` — clean. - `yarn test --include='**/gmail.service.spec.ts'` — 2 passed. - `yarn prettier --check src/app/common/service/gmail/gmail.service.spec.ts` — clean. Manual verification path (for reviewers reproducing the issue's repro): 1. Don't set `USER_SYS_GOOGLE_SMTP_GMAIL` / `USER_SYS_GOOGLE_SMTP_PASSWORD`. 2. Start amber backend + frontend. 3. Open the share dialog on any workflow/dataset/project, enter a real email, submit. 4. Frontend: red toast *"Failed to send email. Please try again or contact admin."*. 5. Backend log: `Failed to send email: 535-5.7.8 Username and Password not accepted...` (unchanged). 6. Browser console: `Send email error: ...` (the specific SMTP error). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-7) --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> --- .../apache/texera/web/resource/GmailResource.scala | 9 ++- .../texera/web/resource/GmailResourceSpec.scala | 74 ++++++++++++++++++++++ .../app/common/service/gmail/gmail.service.spec.ts | 66 +++++++++++++++++++ .../src/app/common/service/gmail/gmail.service.ts | 3 +- 4 files changed, 150 insertions(+), 2 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala index f06f1f9210..ab91c9ad43 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/GmailResource.scala @@ -33,6 +33,7 @@ import javax.annotation.security.RolesAllowed import javax.mail.internet.{InternetAddress, MimeMessage} import javax.mail.{Message, PasswordAuthentication, Session, Transport} import javax.ws.rs._ +import javax.ws.rs.core.Response import scala.util.{Failure, Success, Try} case class EmailMessage( @@ -146,7 +147,13 @@ class GmailResource { @Path("/send") def sendEmailRequest(emailMessage: EmailMessage, @Auth user: SessionUser): Unit = { val recipientEmail = if (emailMessage.receiver.isEmpty) user.getEmail else emailMessage.receiver - sendEmail(emailMessage, recipientEmail) + sendEmail(emailMessage, recipientEmail) match { + case Right(_) => () + case Left("Invalid email format") => + throw new BadRequestException("Invalid email format") + case Left(error) => + throw new WebApplicationException(error, Response.Status.BAD_GATEWAY) + } } @GET diff --git a/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala new file mode 100644 index 0000000000..868b0e34cd --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/GmailResourceSpec.scala @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.web.resource + +import org.apache.texera.auth.SessionUser +import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.flatspec.AnyFlatSpec + +import javax.ws.rs.{BadRequestException, WebApplicationException} + +class GmailResourceSpec extends AnyFlatSpec { + + private def newSessionUser(): SessionUser = { + val user = new User + user.setUid(Integer.valueOf(1)) + user.setName("test") + user.setRole(UserRoleEnum.REGULAR) + user.setEmail("[email protected]") + new SessionUser(user) + } + + it should "throw BadRequestException (HTTP 400) when the receiver fails email-format validation" in { + val resource = new GmailResource() + val msg = EmailMessage( + receiver = "not-a-valid-email", + subject = "subj", + content = "body" + ) + val ex = intercept[BadRequestException] { + resource.sendEmailRequest(msg, newSessionUser()) + } + assert(ex.getResponse.getStatus == 400) + } + + it should "throw WebApplicationException with HTTP 502 when sendEmail fails for a non-validation reason" in { + // In the test environment `UserSystemConfig.gmail` defaults to "", so + // `createMimeMessage`'s `new InternetAddress(senderGmail)` raises an + // `AddressException` deterministically — without any network or SMTP + // server contact — and `sendEmail` returns `Left("Failed to send email: + // ...")`. The resource then maps that `Left` to a 502 BadGateway. + val resource = new GmailResource() + val msg = EmailMessage( + receiver = "[email protected]", + subject = "subj", + content = "body" + ) + val ex = intercept[WebApplicationException] { + resource.sendEmailRequest(msg, newSessionUser()) + } + assert( + !ex.isInstanceOf[BadRequestException], + s"expected non-validation failure, but got BadRequestException: ${ex.getMessage}" + ) + assert(ex.getResponse.getStatus == 502) + } +} diff --git a/frontend/src/app/common/service/gmail/gmail.service.spec.ts b/frontend/src/app/common/service/gmail/gmail.service.spec.ts new file mode 100644 index 0000000000..fd4ce18ca7 --- /dev/null +++ b/frontend/src/app/common/service/gmail/gmail.service.spec.ts @@ -0,0 +1,66 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { TestBed } from "@angular/core/testing"; +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { GmailService } from "./gmail.service"; +import { NotificationService } from "../notification/notification.service"; + +describe("GmailService", () => { + let service: GmailService; + let httpTestingController: HttpTestingController; + let notificationSpy: { success: ReturnType<typeof vi.fn>; error: ReturnType<typeof vi.fn> }; + + beforeEach(() => { + notificationSpy = { success: vi.fn(), error: vi.fn() }; + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [GmailService, { provide: NotificationService, useValue: notificationSpy }], + }); + service = TestBed.inject(GmailService); + httpTestingController = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpTestingController.verify(); + }); + + it("should show a success toast when the backend accepts the send request", () => { + service.sendEmail("subj", "body", "[email protected]"); + + const req = httpTestingController.expectOne(r => r.url.endsWith("/gmail/send") && r.method === "PUT"); + req.flush(null); + + expect(notificationSpy.success).toHaveBeenCalledWith("Email sent successfully"); + expect(notificationSpy.error).not.toHaveBeenCalled(); + }); + + it("should show an error toast when the backend returns an HTTP error (e.g. SMTP failure)", () => { + service.sendEmail("subj", "body", "[email protected]"); + + const req = httpTestingController.expectOne(r => r.url.endsWith("/gmail/send") && r.method === "PUT"); + req.flush("Failed to send email: 535-5.7.8 Username and Password not accepted", { + status: 502, + statusText: "Bad Gateway", + }); + + expect(notificationSpy.error).toHaveBeenCalledWith("Failed to send email. Please try again or contact admin."); + expect(notificationSpy.success).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/app/common/service/gmail/gmail.service.ts b/frontend/src/app/common/service/gmail/gmail.service.ts index 925e7e3a39..3dfec89a06 100644 --- a/frontend/src/app/common/service/gmail/gmail.service.ts +++ b/frontend/src/app/common/service/gmail/gmail.service.ts @@ -42,7 +42,8 @@ export class GmailService { next: () => this.notificationService.success("Email sent successfully"), error: (error: unknown) => { if (error instanceof HttpErrorResponse) { - console.error(error.error); + this.notificationService.error("Failed to send email. Please try again or contact admin."); + console.error("Send email error:", error.error); } }, });
