fix: standardize API error handling across services and composables by NickGhignatti · Pull Request #137 · NickGhignatti/crowd-vision

NickGhignatti · 2026-03-28T21:55:11Z

Replace throw-on-HTTP-error patterns with early returns throughout the client composables and Pinia stores, aligning error handling with a consistent log-and-return strategy instead of propagating exceptions to callers for expected API failures.

Changes

useAuthStore: login and register now return early on non-ok responses instead of throwing, keeping error handling internal to the action
Rename usePush export to useWebPushNotifications for consistency
Updated tests which expected to throw errors
Create a standard openapi file for the errors
Performance update on backend microservices

Close #136
Enhance #133

Copilot

Pull request overview

This PR standardizes error handling across backend microservices (typed errors + global Express error middleware) and updates client composables/stores and OpenAPI docs to follow a consistent “handle expected API failures without throwing” approach.

Changes:

Introduce typed BaseError classes + globalErrorHandler middleware in multiple services, and refactor controllers/services to throw typed errors instead of ad-hoc Error/per-controller try/catch.
Update client Pinia stores and composables to return early (log + no throw) on non-OK API responses; rename push composable export to useWebPushNotifications.
Add shared OpenAPI error spec (landing-page/api/error.yaml) and refactor service OpenAPI docs to reference shared error responses; update Jest MongoDB test setup for parallelism.

Reviewed changes

Copilot reviewed 55 out of 56 changed files in this pull request and generated 23 comments.

Show a summary per file

File	Description
server/twin-service/src/services/twinService.ts	Replace generic errors with typed `ConflictError`/`NotFoundError`; reuse `getBuildingById` in update.
server/twin-service/src/models/error.ts	Add typed error classes with HTTP status codes.
server/twin-service/src/middlewares/errorsMiddleware.ts	Add global error handler to serialize typed errors.
server/twin-service/src/index.ts	Register global error handler; remove swagger wiring (but swagger imports remain).
server/twin-service/src/controller/twinController.ts	Remove per-handler try/catch and rely on global error middleware.
server/twin-service/package.json	Add Jest `globalSetup`/`globalTeardown`.
server/twin-service/tests/twin.test.ts	Update expectations for new status codes/payload shape.
server/twin-service/tests/setup.ts	Switch to shared MongoMemoryServer via global setup and per-worker DB.
server/twin-service/tests/globalSetup.cjs	Start MongoMemoryServer once per Jest run.
server/twin-service/tests/globalTeardown.cjs	Stop MongoMemoryServer once per Jest run.
server/notification-service/src/services/pushService.ts	Throw `NotFoundError` when no subscriptions rather than logging/returning.
server/notification-service/src/services/notificationService.ts	Remove try/catch/logging around Redis publish; keep periodic loop.
server/notification-service/src/models/error.ts	Add typed error classes with HTTP status codes.
server/notification-service/src/middlewares/errorsMiddleware.ts	Add global error handler to serialize typed errors.
server/notification-service/src/index.ts	Register global error handler; remove swagger wiring (but swagger imports remain).
server/notification-service/src/controller/notificationController.ts	Replace inline 400/500 responses with typed errors for validation.
server/notification-service/package.json	Add Jest `globalSetup`/`globalTeardown`.
server/notification-service/openapi.yaml	Remove service-local OpenAPI spec.
server/notification-service/tests/setup.ts	Switch to shared MongoMemoryServer via global setup and per-worker DB.
server/notification-service/tests/notification.test.ts	Update mocked typing + new error response shape assertions.
server/notification-service/tests/globalSetup.cjs	Start MongoMemoryServer once per Jest run.
server/notification-service/tests/globalTeardown.cjs	Stop MongoMemoryServer once per Jest run.
server/auth-service/src/services/totpService.ts	Refactor OTP role resolution to throw typed errors instead of returning null.
server/auth-service/src/services/tokenService.ts	Replace generic errors with typed errors for missing secret/account.
server/auth-service/src/services/domainService.ts	Replace generic errors with typed errors; formatting/typing tweaks.
server/auth-service/src/services/authenticationService.ts	Replace generic errors with typed errors across auth/SSO flows.
server/auth-service/src/router.ts	Update role middleware import path.
server/auth-service/src/models/role.ts	Introduce new role/authorization middleware module.
server/auth-service/src/models/error.ts	Add typed error classes with HTTP status codes.
server/auth-service/src/models/domain.ts	Update role import path.
server/auth-service/src/middlewares/errorsMiddleware.ts	Add global error handler to serialize typed errors.
server/auth-service/src/index.ts	Register global error handler (but still mounts swagger route).
server/auth-service/src/controller/domainController.ts	Remove per-handler try/catch and rely on global error middleware.
server/auth-service/src/controller/authenticationMiddleware.ts	Convert auth/hmac failures to typed throws.
server/auth-service/src/controller/authenticationController.ts	Remove per-handler try/catch and rely on global error middleware (incl. SSO callback).
server/auth-service/src/controller/administrationController.ts	Remove try/catch; adjust enterprise provisioning flow.
server/auth-service/package.json	Add Jest `globalSetup`/`globalTeardown`.
server/auth-service/openapi.yaml	Remove service-local OpenAPI spec.
server/auth-service/tests/setup.ts	Switch to shared MongoMemoryServer via global setup and per-worker DB.
server/auth-service/tests/globalSetup.cjs	Start MongoMemoryServer once per Jest run.
server/auth-service/tests/globalTeardown.cjs	Stop MongoMemoryServer once per Jest run.
server/auth-service/tests/domain.test.ts	Update domain/subdomain expectations.
landing-page/index.html	Add link to shared error OpenAPI spec.
landing-page/api/twin.yaml	Reference shared error responses.
landing-page/api/notification.yaml	Reference shared error responses.
landing-page/api/error.yaml	Add shared error response/spec definitions.
landing-page/api/auth.yaml	Reference shared error responses (but introduces indentation issues).
client/src/stores/domain.ts	Return empty lists on non-OK responses instead of using error payload.
client/src/stores/authentication.ts	Stop throwing on non-OK login/register; log and early return.
client/src/stores/tests/authentication.spec.ts	Update expectations to “no throw” behavior.
client/src/composables/useWebPushNotifications.ts	Rename composable + switch to `makeRequest`; log-and-return on errors.
client/src/composables/useBuildingModel.ts	Remove outdated comment.
client/src/composables/tests/useWebPushNotifications.spec.ts	Update imports/name and expectations to match new composable.
client/src/components/cards/tests/DomainCard.spec.ts	Update mocked helper path + expected role text (currently inconsistent).
client/src/components/tests/PushNotificationToast.spec.ts	Update mock target to new composable name.
client/src/components/PushNotificationToast.vue	Update to new composable name/import.

Comments suppressed due to low confidence (1)

server/notification-service/src/services/notificationService.ts:19

setInterval(async () => { ... }) does not handle rejected promises. With the changes in this PR (e.g., sendPushToAll now throws on no subscriptions and publishNotification no longer catches Redis failures), this interval callback can trigger unhandled promise rejections (and potentially terminate the process depending on Node settings). Wrap the interval body in a try/catch (and decide which errors are expected vs. fatal) to keep the loop robust.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T22:02:35Z

server/auth-service/src/index.ts

 app.use("/", router);
 app.use("/api-docs", swaggerUi.serve, swaggerUi.setup(swaggerDocument));
+app.use(globalErrorHandler);


swaggerDocument is loaded from ./openapi.yaml and /api-docs is still mounted, but this PR deletes server/auth-service/openapi.yaml. This will crash on startup when YAML.load runs. Either restore/replace the service-local OpenAPI file, or remove the YAML load + /api-docs wiring from this service to match the other services.

Copilot · 2026-03-28T22:02:35Z

server/auth-service/src/middlewares/errorsMiddleware.ts

+    return res.status(err.code).json({
+      type: err.type,
+      message: err.message,
+    });
+  }
+
+  return res.status(500).json({


The standardized error response produced here only returns { type, message }, but the new shared OpenAPI error contract (landing-page/api/error.yaml) requires code as well. To keep runtime responses aligned with the published contract, include an explicit code field in this JSON (and ensure its type matches the OpenAPI schema), or update the shared contract to match what the services actually return.

Suggested change

return res.status(err.code).json({

type: err.type,

message: err.message,

});

}

return res.status(500).json({

return res.status(err.code).json({

code: err.code,

type: err.type,

message: err.message,

});

}

return res.status(500).json({

code: 500,

Copilot · 2026-03-28T22:02:35Z

server/auth-service/src/controller/authenticationMiddleware.ts

  const signature = req.headers["x-signature"];
  const secret = getAdminSecret();

  if (!secret) {
-    return res.status(500).json({ error: "Server configuration error" });
+    throw new InternalError("Server configuration error, missing administration secrets");
  }

  if (!signature || typeof signature !== "string") {
-    return res.status(401).json({ error: "Forbidden: missing signature" });
+    throw new ForbiddenError(`Invalid signature`);


This middleware reads the signature from req.headers["x-signature"], but the OpenAPI spec for business registration documents x-hmac-signature. This mismatch will cause clients to send a header the server ignores. Consider standardizing on one name (preferably the documented one), or temporarily accepting both headers for backward compatibility.

Copilot · 2026-03-28T22:02:36Z

landing-page/api/twin.yaml

-                properties:
-                  error:
-                    type: string
+          $ref: "error.yaml#/components/responses/BadRequest"


Twin service POST /register now throws ConflictError (HTTP 409) when a building already exists, but this OpenAPI doc only defines a 400 error response. Add a 409/Conflict response (and define it in error.yaml if needed) so the spec matches runtime behavior.

Suggested change

$ref: "error.yaml#/components/responses/BadRequest"

$ref: "error.yaml#/components/responses/BadRequest"

"409":

$ref: "error.yaml#/components/responses/Conflict"

Copilot · 2026-03-28T22:02:36Z

client/src/components/cards/__tests__/DomainCard.spec.ts

 }))

-vi.mock('@/helpers/roles.ts', () => ({
+vi.mock('@/helpers/role.ts', () => ({


DomainCard.vue imports getRoleMeta from @/helpers/roles.ts, but this test mocks @/helpers/role.ts. The mock won't apply, and the test will use the real helper (or fail to resolve the mocked module). Update the mocked path to match the component import.

Suggested change

vi.mock('@/helpers/role.ts', () => ({

vi.mock('@/helpers/roles.ts', () => ({

Copilot · 2026-03-28T22:02:39Z

server/auth-service/src/controller/authenticationController.ts

+  // Pass full URL to service to handle state/code extraction
+  const clientRedirect = await AuthService.processSSOCallback(req.originalUrl);
+  res.redirect(clientRedirect);


handleSSOCallback no longer catches errors from processSSOCallback. Previously it always redirected the browser back to the client with an error=sso_failed query param; now failures will be handled by the global error middleware (JSON 4xx/5xx), which breaks the expected browser redirect flow from the IdP callback. Consider restoring the try/catch + redirect-on-failure behavior for this endpoint, even if other endpoints rely on global error handling.

Suggested change

// Pass full URL to service to handle state/code extraction

const clientRedirect = await AuthService.processSSOCallback(req.originalUrl);

res.redirect(clientRedirect);

try {

// Pass full URL to service to handle state/code extraction

const clientRedirect = await AuthService.processSSOCallback(req.originalUrl);

res.redirect(clientRedirect);

} catch (error) {

// Ensure SSO callback always results in a browser redirect, even on failure

// Redirect back to client with a standard error indicator

console.error("SSO callback processing failed:", error);

res.redirect("/?error=sso_failed");

}

Copilot · 2026-03-28T22:02:40Z

server/auth-service/src/services/totpService.ts

+  if (!domains || domains.length <= 0) {
+    throw new NotFoundError("Domain or SSO configuration not found");
+  }


resolveRoleFromOTP throws NotFoundError (404) for cases like "no domains configured" and "invalid OTP". These are validation/configuration problems rather than missing resources, and 404 will be confusing for clients (and for the shared error responses). Consider using ValidationError for an invalid OTP (400) and a distinct server/config error (500) when TOTP/SSO configuration is missing.

Copilot · 2026-03-28T22:02:40Z

client/src/stores/__tests__/authentication.spec.ts

    it('throws when the response is not ok', async () => {
-      vi.mocked(makeRequest).mockResolvedValue(makeResponse(false) as unknown as Response)
-
-      await expect(useAuthStore().register('bob', 'bob@example.com', 'pass')).rejects.toThrow(
-        'Registration failed',
+      vi.mocked(makeRequest).mockResolvedValue(
+        makeResponse(false, {
+          type: 'AuthError',
+          message: 'User already exists',
+        }) as unknown as Response,
      )
+      await expect(
+        useAuthStore().register('bob', 'bob@example.com', 'pass'),
+      ).resolves.toBeUndefined()


This test case name still says "throws when the response is not ok", but the updated expectations now assert the action resolves without throwing. Rename the test to reflect the new contract (early return / no state mutation) to avoid misleading future readers.

Copilot · 2026-03-28T22:02:40Z

landing-page/api/auth.yaml

              schema:
                $ref: "#/components/schemas/Account"
        "401":
-          description: Unauthorized
+            $ref: "error.yaml#/components/responses/Unauthorized"


Same indentation issue for the 401 response under /me: $ref is indented too far, which can invalidate the OpenAPI YAML structure. Ensure $ref is at the correct level directly under the "401" response object.

Copilot · 2026-03-28T22:02:40Z

server/auth-service/src/controller/administrationController.ts

-    res.status(500).json({ message: "Error creating administrator account", error: error.message });
+  if (!enterpriseDomain) {
+    throw new NotFoundError(`Enterprise domain with name "${companyName}" not found`);
  }


provideEnterpriseAdministratorAccount creates an account and looks up the enterprise domain, but on the success path it never sends a response (no res.status(...).json(...), res.send(), or next()). This will leave the HTTP request hanging. Add a success response (and any missing domain membership wiring) or remove the unfinished endpoint until it is implemented.

Suggested change

}

}

const token = await generateStandardToken({

accountId: createdAccount._id.toString(),

accountName: createdAccount.name,

} as StandardTokenPayload);

res.status(201).json({

message: `Administrator account "${createdAccount.name}" for company "${companyName}" created successfully`,

token,

account: {

accountName: createdAccount.name,

},

});

Copilot

Pull request overview

Copilot reviewed 62 out of 64 changed files in this pull request and generated 12 comments.

Comments suppressed due to low confidence (1)

server/notification-service/src/services/notificationService.ts:19

The notification loop runs an async function inside setInterval without any error handling. With the new behavior (e.g., sendPushToAll throwing when there are no subscriptions, Redis publish failures, etc.), this can create unhandled promise rejections and potentially crash the process or spam logs. Wrap the interval body in a try/catch (and/or ensure sendPushToAll returns gracefully for expected conditions) so the loop is resilient.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T22:23:36Z

server/auth-service/src/services/tokenService.ts

+import jwt, { type JwtPayload } from "jsonwebtoken";



JwtPayload is imported but not used. Removing it will keep the module tidy and avoid confusion about expected return types from verifyToken.

Copilot · 2026-03-28T22:23:36Z

landing-page/api/auth.yaml

        "401":
-          description: Unauthorized
+            $ref: "error.yaml#/components/responses/Unauthorized"



Same indentation issue for the "401" response under /me: $ref is over-indented, making the OpenAPI YAML invalid.

Copilot · 2026-03-28T22:23:36Z

server/auth-service/src/controller/authenticationMiddleware.ts

+
+  req.account = TokenService.verifyToken(token);
+  next();


TokenService.verifyToken can throw (expired/invalid JWT). With the current code that error will bubble into the global handler as a 500, which is incorrect for auth failures. Wrap verification in a try/catch and rethrow an UnauthorizedError (or similar) so invalid/expired cookies return 401.

Copilot · 2026-03-28T22:23:37Z

server/auth-service/src/services/authenticationService.ts

  const account = await Account.findOne({ name: accountName });
+
  if (!account) {
-    throw new Error("wrong account name");
+    throw new NotFoundError("Account with this name doesn't exist");
  }


For login, returning a distinct NotFoundError (404) when the account doesn’t exist diverges from the OpenAPI contract (401) and makes user enumeration easier. Consider using a uniform UnauthorizedError for invalid credentials instead.

Copilot · 2026-03-28T22:23:37Z

server/auth-service/src/services/totpService.ts

+  if (!domains || domains.length <= 0) {
+    throw new NotFoundError("Domain or SSO configuration not found");
+  }


If no domains/TOTP secrets are configured, this is a server-side configuration/state problem rather than a missing resource requested by the client. Returning NotFoundError (404) here is misleading; consider using a 500-class error (e.g., InternalError).

Copilot · 2026-03-28T22:23:37Z

server/auth-service/__tests__/domain.test.ts

+      expect(domain.subdomains).toHaveLength(0);

      const subdomainNames = await getDomainSubdomains(mockDomain.name);
-      expect(subdomainNames).toEqual(
-        expect.arrayContaining([
-          `studio.${mockDomain.name}`,
-          `docenti.${mockDomain.name}`,
-        ]),
-      );
+      expect(subdomainNames).toEqual(expect.arrayContaining([]));
    });


This test no longer creates any subdomains, and the assertion arrayContaining([]) will always pass. This weakens coverage for the getDomainSubdomains graph lookup behavior. Consider restoring creation/attachment of real subdomain documents and asserting that their names are returned.

Copilot · 2026-03-28T22:23:37Z

server/auth-service/src/services/totpService.ts

+  throw new NotFoundError("Invalid OTP provided");
 };


An invalid OTP is not a “not found” resource. Returning NotFoundError (404) for bad OTPs is misleading for clients; consider a 400/401-class error (e.g., ValidationError/UnauthorizedError).

Copilot · 2026-03-28T22:23:38Z

landing-page/api/error.yaml

+      required:
+        - type
+        - code
+        - message
+      properties:


Error schema requires a code field (string), but the server error middleware currently returns only { type, message } and uses numeric codes as HTTP status. Either add a machine-readable code field to error responses, or update this schema to match actual responses (and don’t require code if it’s not returned).

Copilot · 2026-03-28T22:23:38Z

server/twin-service/src/index.ts

 import swaggerUi from 'swagger-ui-express';
 import YAML from "yamljs";


After removing Swagger setup/routes from this service, swagger-ui-express and yamljs are now unused imports. Consider removing them to avoid loading unnecessary dependencies at runtime.

Copilot · 2026-03-28T22:23:38Z

server/auth-service/src/services/authenticationService.ts

  const match = await bcrypt.compare(password, account.password);
  if (!match) {
-    throw new Error("wrong password");
+    throw new ValidationError("Invalid password");
  }


Invalid passwords currently throw ValidationError (400). For authentication endpoints this should typically be UnauthorizedError (401) to match the API contract and avoid leaking which part of the credentials was wrong.

NickGhignatti · 2026-03-28T22:30:54Z

🎉 This PR is included in version 2.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

NickGhignatti added 3 commits March 26, 2026 21:32

feat: errors openapi spec

11be756

feat: error handler

971c1f8

fix: remove fetch for using the useApi composable

52467df

NickGhignatti self-assigned this Mar 28, 2026

Copilot AI review requested due to automatic review settings March 28, 2026 21:55

Copilot started reviewing on behalf of NickGhignatti March 28, 2026 21:55 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings March 28, 2026 22:15

Copilot started reviewing on behalf of NickGhignatti March 28, 2026 22:16 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

fix: test with the new way of managing errors

e2776d7

NickGhignatti force-pushed the feat/errors-standardization branch from 490cf9a to e2776d7 Compare March 28, 2026 22:28

NickGhignatti merged commit a76436b into master Mar 28, 2026
13 checks passed

NickGhignatti deleted the feat/errors-standardization branch March 28, 2026 22:30

NickGhignatti added the released label Mar 28, 2026

	vi.mock('@/helpers/role.ts', () => ({
	vi.mock('@/helpers/roles.ts', () => ({

-  }
+  }
+  const token = await generateStandardToken({
+    accountId: createdAccount._id.toString(),
+    accountName: createdAccount.name,
+  } as StandardTokenPayload);
+  res.status(201).json({
+    message: `Administrator account "${createdAccount.name}" for company "${companyName}" created successfully`,
+    token,
+    account: {
+      accountName: createdAccount.name,
+    },
+  });

		import swaggerUi from 'swagger-ui-express';
		import YAML from "yamljs";

Conversation

NickGhignatti commented Mar 28, 2026

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NickGhignatti commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects