diff --git a/ccm_web/server/src/api/api.admin.handler.ts b/ccm_web/server/src/api/api.admin.handler.ts index 65e2786f7..2f3dfd411 100644 --- a/ccm_web/server/src/api/api.admin.handler.ts +++ b/ccm_web/server/src/api/api.admin.handler.ts @@ -3,6 +3,7 @@ import CanvasRequestor from '@kth/canvas-api' import { CourseApiHandler } from './api.course.handler' import { APIErrorData, isAPIErrorData } from './api.interfaces' import { + checkForUniqueIdError, createLimitedPromises, handleAPIError, HttpMethod, @@ -119,7 +120,7 @@ export class AdminApiHandler { return finalResult } - async createExternalUser (user: ExternalUserDto, accountID: number): Promise { + async createExternalUser (user: ExternalUserDto, accountID: number): Promise { const email = user.email const loginId = email.replace('@', '+') const fullName = `${user.givenName} ${user.surname}` @@ -158,6 +159,7 @@ export class AdminApiHandler { } = response.body return { id, name, sortable_name, short_name, login_id, email } } catch (error: unknown) { + if (checkForUniqueIdError(error)) return false const errorResponse = handleAPIError(error, loginId) return { statusCode: errorResponse.canvasStatusCode, @@ -168,7 +170,7 @@ export class AdminApiHandler { async createExternalUsers ( users: ExternalUserDto[], accountID: number - ): Promise> { + ): Promise> { const start = process.hrtime.bigint() // Try creating all Canvas users; failure often means user already exists diff --git a/ccm_web/server/src/api/api.service.ts b/ccm_web/server/src/api/api.service.ts index 6b0e99bf8..5a9eaeaad 100644 --- a/ccm_web/server/src/api/api.service.ts +++ b/ccm_web/server/src/api/api.service.ts @@ -1,5 +1,5 @@ import { SessionData } from 'express-session' -import { HttpStatus, Injectable } from '@nestjs/common' +import { Injectable } from '@nestjs/common' import { ConfigService } from '@nestjs/config' import { AdminApiHandler } from './api.admin.handler' @@ -146,19 +146,12 @@ export class APIService { // Handle create user responses: success, failure, and already exists createUserResponses.forEach(({ email, result }) => { - let userCreated: false | APIErrorData | CanvasUserLoginEmail if (isAPIErrorData(result)) { - if (result.statusCode === HttpStatus.BAD_REQUEST) { - userCreated = false - } else { - userCreated = result - createErrors.push(result) - } - } else { - userCreated = result + createErrors.push(result) + } else if (result !== false) { newUsers.push(result) } - resultData[email] = { userCreated } + resultData[email] = { userCreated: result } }) if (createErrors.length === externalUsers.length) { const statusCode = determineStatusCode(createErrors.map(e => e.statusCode)) diff --git a/ccm_web/server/src/api/api.utils.ts b/ccm_web/server/src/api/api.utils.ts index 4a7049b1d..df6c7e69b 100644 --- a/ccm_web/server/src/api/api.utils.ts +++ b/ccm_web/server/src/api/api.utils.ts @@ -5,7 +5,7 @@ import pLimit from 'p-limit' import { APIErrorData, APIErrorPayload, isAPIErrorData } from './api.interfaces' -import { isCanvasErrorBody } from '../canvas/canvas.interfaces' +import { isCanvasMessageErrorBody, isCanvasMessageErrorsBody, isCanvasUniqueIdErrorsBody } from '../canvas/canvas.interfaces' import baseLogger from '../logger' @@ -20,13 +20,43 @@ export enum HttpMethod { Delete = 'DELETE' } +export function checkForUniqueIdError (error: unknown): boolean { + if (!(error instanceof CanvasApiError && error.response !== undefined)) return false + const { statusCode, body } = error.response + logger.debug('Checking if an error was thrown because the user already exists...') + logger.debug(JSON.stringify(body, null, 2)) + return ( + statusCode === HttpStatus.BAD_REQUEST && + isCanvasUniqueIdErrorsBody(body) && + body.errors.pseudonym.unique_id.length > 0 && + body.errors.pseudonym.unique_id[0].type === 'taken' + ) +} + +function parseErrorBody (body: unknown): string { + if (body === null || body === undefined || String(body).startsWith('')) return 'No response body was found.' + if (isCanvasMessageErrorBody(body)) { + return body.message + } else if (isCanvasMessageErrorsBody(body)) { + return body.errors.map(e => e.message).join(' ') + } else if (isCanvasUniqueIdErrorsBody(body)) { + return ( + body.errors.pseudonym.unique_id.length > 0 + ? body.errors.pseudonym.unique_id[0].message + : 'Unique ID error had no message.' + ) + } else { + return `Canvas response body had unhandled shape: ${JSON.stringify(body)}` + } +} + export function handleAPIError (error: unknown, input?: string): APIErrorPayload { const failedInput = input === undefined ? null : input if (error instanceof CanvasApiError && error.response !== undefined) { const { statusCode, body } = error.response const bodyText = parseErrorBody(body) logger.error(`Received error status code: (${String(statusCode)})`) - logger.error(`Response body: (${bodyText})`) + logger.error(`Response message(s): (${bodyText})`) logger.error(`Failed input: (${String(failedInput)})`) return { canvasStatusCode: statusCode, message: bodyText, failedInput: failedInput } } else { @@ -36,21 +66,6 @@ export function handleAPIError (error: unknown, input?: string): APIErrorPayload } } -export function parseErrorBody (body: unknown): string { - if (body === null || body === undefined || String(body).startsWith('')) return 'No response body was found.' - if (!isCanvasErrorBody(body)) { - return `Response body had unexpected shape: ${JSON.stringify(body)}` - } - let errorMessage: string - try { - errorMessage = body.errors.map(e => e.message).join(' ') - } catch (e) { - errorMessage = JSON.stringify(body) - logger.debug(errorMessage) - } - return errorMessage -} - export function determineStatusCode (codes: HttpStatus[]): HttpStatus { if (codes.length === 0) throw new Error('determineStatusCode received an array with length 0.') const uniqueStatusCodes: Set = new Set(codes) diff --git a/ccm_web/server/src/canvas/canvas.interfaces.ts b/ccm_web/server/src/canvas/canvas.interfaces.ts index bf637f3be..6288720c1 100644 --- a/ccm_web/server/src/canvas/canvas.interfaces.ts +++ b/ccm_web/server/src/canvas/canvas.interfaces.ts @@ -142,33 +142,65 @@ export interface CourseWithSections extends CanvasCourseBase { sections: CanvasCourseSection[] } -// Errors +// Error Data -interface CanvasError { +export interface CanvasMessageErrorBody { message: string } -function isCanvasError (value: unknown): value is CanvasError { +export function isCanvasMessageErrorBody (value: unknown): value is CanvasMessageErrorBody { return hasKeys(value, ['message']) } -export interface CanvasErrorBody { - errors: CanvasError[] +export interface CanvasErrorsBody { + errors: unknown } -export function isCanvasErrorBody (value: unknown): value is CanvasErrorBody { - if (!hasKeys(value, ['errors'])) { - return false - } +export function isCanvasErrorsBody (value: unknown): value is CanvasErrorsBody { + return hasKeys(value, ['errors']) +} + +export interface CanvasMessageErrorsBody extends CanvasErrorsBody { + errors: CanvasMessageErrorBody[] +} + +export function isCanvasMessageErrorsBody (value: unknown): value is CanvasMessageErrorsBody { + if (!isCanvasErrorsBody(value)) return false + return ( + Array.isArray(value.errors) && + value.errors.every(e => isCanvasMessageErrorBody(e)) + ) +} - if (Array.isArray(value.errors)) { - const result = value.errors.map(e => isCanvasError(e)).every(e => e) - return result - } else { - return true +interface UniqueIdErrorData { + attribute: 'unique_id' + type: string + message: string +} + +export interface CanvasUniqueIdErrorsBody extends CanvasErrorsBody { + errors: { + pseudonym: { + unique_id: UniqueIdErrorData[] + } } } +export function isCanvasUniqueIdErrorsBody (value: unknown): value is CanvasUniqueIdErrorsBody { + if (!isCanvasErrorsBody(value)) return false + return ( + hasKeys(value.errors, ['pseudonym']) && + hasKeys(value.errors.pseudonym, ['unique_id']) && + Array.isArray(value.errors.pseudonym.unique_id) && + value.errors.pseudonym.unique_id.every(o => { + return ( + hasKeys(o, ['attribute', 'type', 'message']) && + Object.values(o).every(v => typeof v === 'string') + ) + }) + ) +} + export const isOAuthErrorResponseQuery = (value: unknown): value is OAuthErrorResponseQuery => { return hasKeys(value, ['error']) }