This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 0a42fcdbc2 fix: surface SMTP failures from gmail share endpoint to UI
(#5164)
0a42fcdbc2 is described below
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);
}
},
});
