🪟 🐛 Fix job failures not showing when discover schema fails while cre…

…ating a connection (#15624) * Fix status check on `JobItem` and ensure that `SourceService` returns `LogsRequestError` when there are logs. * Remove `SynchronousJobReadWithStatus` interface, no longer needed. * Fix conditional check in JobItem/MainInfo to prevent it from rendering 0 when there are no attempts
airbytehq · Aug 17, 2022 · 2190383 · 2190383
1 parent 8c79e4f
commit 2190383
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 41 deletions.
diff --git a/airbyte-webapp/src/components/JobItem/JobItem.tsx b/airbyte-webapp/src/components/JobItem/JobItem.tsx
@@ -3,10 +3,9 @@ import styled from "styled-components";
 
 import { Spinner } from "components";
 
-import { SynchronousJobReadWithStatus } from "core/request/LogsRequestError";
 import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/components/JobsList";
 
-import { AttemptRead, CheckConnectionReadStatus, JobStatus } from "../../core/request/AirbyteClient";
+import { AttemptRead, JobStatus, SynchronousJobRead } from "../../core/request/AirbyteClient";
 import { useAttemptLink } from "./attemptLinkUtils";
 import ContentWrapper from "./components/ContentWrapper";
 import ErrorDetails from "./components/ErrorDetails";
@@ -31,24 +30,20 @@ const LoadLogs = styled.div`
 `;
 
 interface JobItemProps {
-  job: SynchronousJobReadWithStatus | JobsWithJobs;
+  job: SynchronousJobRead | JobsWithJobs;
 }
 
-const didJobSucceed = (job: SynchronousJobReadWithStatus | JobsWithJobs) => {
-  return getJobStatus(job) !== "failed";
-};
+const didJobSucceed = (job: SynchronousJobRead | JobsWithJobs): boolean =>
+  "succeeded" in job ? job.succeeded : getJobStatus(job) !== "failed";
 
-export const getJobStatus: (
-  job: SynchronousJobReadWithStatus | JobsWithJobs
-) => JobStatus | CheckConnectionReadStatus = (job) => {
-  return "status" in job ? job.status : job.job.status;
-};
+export const getJobStatus: (job: SynchronousJobRead | JobsWithJobs) => JobStatus = (job) =>
+  "succeeded" in job ? (job.succeeded ? JobStatus.succeeded : JobStatus.failed) : job.job.status;
 
-export const getJobAttemps: (job: SynchronousJobReadWithStatus | JobsWithJobs) => AttemptRead[] | undefined = (job) => {
-  return "attempts" in job ? job.attempts : undefined;
-};
+export const getJobAttemps: (job: SynchronousJobRead | JobsWithJobs) => AttemptRead[] | undefined = (job) =>
+  "attempts" in job ? job.attempts : undefined;
 
-export const getJobId = (job: SynchronousJobReadWithStatus | JobsWithJobs) => ("id" in job ? job.id : job.job.id);
+export const getJobId = (job: SynchronousJobRead | JobsWithJobs): string | number =>
+  "id" in job ? job.id : job.job.id;
 
 export const JobItem: React.FC<JobItemProps> = ({ job }) => {
   const { jobId: linkedJobId } = useAttemptLink();

diff --git a/airbyte-webapp/src/components/JobItem/components/JobLogs.tsx b/airbyte-webapp/src/components/JobItem/components/JobLogs.tsx
@@ -3,35 +3,32 @@ import React, { useState } from "react";
 import { FormattedMessage } from "react-intl";
 import { useLocation } from "react-router-dom";
 
-import { SynchronousJobReadWithStatus } from "core/request/LogsRequestError";
 import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/components/JobsList";
 import { useGetDebugInfoJob } from "services/job/JobService";
 
-import { AttemptRead, AttemptStatus } from "../../../core/request/AirbyteClient";
+import { AttemptRead, AttemptStatus, SynchronousJobRead } from "../../../core/request/AirbyteClient";
 import { parseAttemptLink } from "../attemptLinkUtils";
 import Logs from "./Logs";
 import { LogsDetails } from "./LogsDetails";
 import Tabs, { TabsData } from "./Tabs";
 
 interface JobLogsProps {
   jobIsFailed?: boolean;
-  job: SynchronousJobReadWithStatus | JobsWithJobs;
+  job: SynchronousJobRead | JobsWithJobs;
 }
 
 const isPartialSuccess = (attempt: AttemptRead) => {
   return !!attempt.failureSummary?.partialSuccess;
 };
 
-const jobIsSynchronousJobRead = (
-  job: SynchronousJobReadWithStatus | JobsWithJobs
-): job is SynchronousJobReadWithStatus => {
-  return !!(job as SynchronousJobReadWithStatus)?.logs?.logLines;
+const jobIsSynchronousJobRead = (job: SynchronousJobRead | JobsWithJobs): job is SynchronousJobRead => {
+  return !!(job as SynchronousJobRead)?.logs?.logLines;
 };
 
 const JobLogs: React.FC<JobLogsProps> = ({ jobIsFailed, job }) => {
   const isSynchronousJobRead = jobIsSynchronousJobRead(job);
 
-  const id: number | string = (job as JobsWithJobs).job?.id ?? (job as SynchronousJobReadWithStatus).id;
+  const id: number | string = (job as JobsWithJobs).job?.id ?? (job as SynchronousJobRead).id;
 
   const debugInfo = useGetDebugInfoJob(id, typeof id === "number", true);
 

diff --git a/airbyte-webapp/src/components/JobItem/components/MainInfo.tsx b/airbyte-webapp/src/components/JobItem/components/MainInfo.tsx
@@ -7,19 +7,18 @@ import { FormattedDateParts, FormattedMessage, FormattedTimeParts } from "react-
 import { StatusIcon } from "components";
 import { Cell, Row } from "components/SimpleTableComponents";
 
-import { AttemptRead, JobStatus } from "core/request/AirbyteClient";
-import { SynchronousJobReadWithStatus } from "core/request/LogsRequestError";
+import { AttemptRead, JobStatus, SynchronousJobRead } from "core/request/AirbyteClient";
 import { JobsWithJobs } from "pages/ConnectionPage/pages/ConnectionItemPage/components/JobsList";
 
 import { getJobStatus } from "../JobItem";
 import AttemptDetails from "./AttemptDetails";
 import styles from "./MainInfo.module.scss";
 
-const getJobConfig = (job: SynchronousJobReadWithStatus | JobsWithJobs) =>
-  (job as SynchronousJobReadWithStatus).configType ?? (job as JobsWithJobs).job.configType;
+const getJobConfig = (job: SynchronousJobRead | JobsWithJobs) =>
+  (job as SynchronousJobRead).configType ?? (job as JobsWithJobs).job.configType;
 
-const getJobCreatedAt = (job: SynchronousJobReadWithStatus | JobsWithJobs) =>
-  (job as SynchronousJobReadWithStatus).createdAt ?? (job as JobsWithJobs).job.createdAt;
+const getJobCreatedAt = (job: SynchronousJobRead | JobsWithJobs) =>
+  (job as SynchronousJobRead).createdAt ?? (job as JobsWithJobs).job.createdAt;
 
 const partialSuccessCheck = (attempts: AttemptRead[]) => {
   if (attempts.length > 0 && attempts[attempts.length - 1].status === JobStatus.failed) {
@@ -29,7 +28,7 @@ const partialSuccessCheck = (attempts: AttemptRead[]) => {
 };
 
 interface MainInfoProps {
-  job: SynchronousJobReadWithStatus | JobsWithJobs;
+  job: SynchronousJobRead | JobsWithJobs;
   attempts?: AttemptRead[];
   isOpen?: boolean;
   onExpand: () => void;
@@ -70,7 +69,7 @@ const MainInfo: React.FC<MainInfoProps> = ({ job, attempts = [], isOpen, onExpan
           ) : (
             <FormattedMessage id={`sources.${getJobStatus(job)}`} />
           )}
-          {attempts.length && (
+          {attempts.length > 0 && (
             <>
               {attempts.length > 1 && (
                 <div className={styles.lastAttempt}>

diff --git a/airbyte-webapp/src/core/domain/connector/SourceService.ts b/airbyte-webapp/src/core/domain/connector/SourceService.ts
@@ -83,7 +83,7 @@ export class SourceService extends AirbyteRequestService {
 
     if (!result.jobInfo?.succeeded || !result.catalog) {
       // @ts-expect-error TODO: address this case
-      const e = new CommonRequestError(result);
+      const e = result.jobInfo?.logs ? new LogsRequestError(result.jobInfo) : new CommonRequestError(result);
       // Generate error with failed status and received logs
       e._status = 400;
       // @ts-expect-error address this case

diff --git a/airbyte-webapp/src/core/request/LogsRequestError.ts b/airbyte-webapp/src/core/request/LogsRequestError.ts
@@ -1,14 +1,10 @@
-import { CheckConnectionReadStatus, SynchronousJobRead } from "./AirbyteClient";
+import { SynchronousJobRead } from "./AirbyteClient";
 import { CommonRequestError } from "./CommonRequestError";
 
-export interface SynchronousJobReadWithStatus extends SynchronousJobRead {
-  status: CheckConnectionReadStatus;
-}
-
 export class LogsRequestError extends CommonRequestError {
   __type = "common.errorWithLogs";
 
-  constructor(private jobInfo: SynchronousJobReadWithStatus, msg?: string) {
+  constructor(private jobInfo: SynchronousJobRead, msg?: string) {
     super(undefined, msg);
     this._status = 400;
   }

diff --git a/airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard.tsx b/airbyte-webapp/src/views/Connector/ConnectorCard/ConnectorCard.tsx
@@ -6,8 +6,8 @@ import { JobItem } from "components/JobItem/JobItem";
 
 import { Action, Namespace } from "core/analytics";
 import { Connector, ConnectorT } from "core/domain/connector";
-import { CheckConnectionRead } from "core/request/AirbyteClient";
-import { LogsRequestError, SynchronousJobReadWithStatus } from "core/request/LogsRequestError";
+import { CheckConnectionRead, SynchronousJobRead } from "core/request/AirbyteClient";
+import { LogsRequestError } from "core/request/LogsRequestError";
 import { useAnalyticsService } from "hooks/services/Analytics";
 import { createFormErrorMessage } from "utils/errorStatusMessage";
 import { ServiceForm, ServiceFormProps, ServiceFormValues } from "views/Connector/ServiceForm";
@@ -25,7 +25,7 @@ export const ConnectorCard: React.FC<
   {
     title?: React.ReactNode;
     full?: boolean;
-    jobInfo?: SynchronousJobReadWithStatus | null;
+    jobInfo?: SynchronousJobRead | null;
   } & Omit<ServiceFormProps, keyof ConnectorCardProvidedProps> &
     (
       | {