Commit 8e505442 authored by Sonia Zorba's avatar Sonia Zorba
Browse files

Improved jobs error handling

parent 85f41989
Loading
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -154,6 +154,16 @@ public class VOSpaceClient {
        });
    }

    public String getErrorDetail(String jobId) {

        HttpRequest request = getRequest("/transfers/" + jobId + "/error")
                .header("Accept", "text/plain")
                .GET()
                .build();

        return call(request, BodyHandlers.ofString(), 200, res -> res);
    }
    
    private <T, U> U call(HttpRequest request, HttpResponse.BodyHandler<T> responseBodyHandler, int expectedStatusCode, Function<T, U> responseHandler) {
        try {
            return httpClient.sendAsync(request, responseBodyHandler)
+32 −5
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ import it.inaf.ia2.vospace.ui.exception.BadRequestException;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import net.ivoa.xml.uws.v1.ErrorType;
import net.ivoa.xml.uws.v1.ExecutionPhase;
import net.ivoa.xml.uws.v1.JobSummary;
import net.ivoa.xml.vospace.v2.Param;
@@ -13,6 +14,8 @@ import net.ivoa.xml.vospace.v2.Protocol;
import net.ivoa.xml.vospace.v2.StructuredDataNode;
import net.ivoa.xml.vospace.v2.Transfer;
import net.ivoa.xml.vospace.v2.View;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.io.ByteArrayResource;
@@ -32,6 +35,8 @@ import org.springframework.web.client.RestTemplate;
@RestController
public class JobController extends BaseController {

    private static final Logger LOG = LoggerFactory.getLogger(JobController.class);
    
    @Value("${vospace-authority}")
    private String authority;

@@ -70,12 +75,34 @@ public class JobController extends BaseController {

        JobSummary job = client.startTransferJob(transfer);
        
        if (job.getPhase() == ExecutionPhase.QUEUED || job.getPhase() == ExecutionPhase.PENDING) {
        if (job.getPhase() == ExecutionPhase.QUEUED
                || job.getPhase() == ExecutionPhase.PENDING
                || job.getPhase() == ExecutionPhase.EXECUTING) {
            return ResponseEntity.ok(new Job(job));
        }
        // TODO: proper handling
        throw new RuntimeException("Error while executing job " + job.getJobId() + ". Job phase is "
                + job.getPhase() + ". QUEUED or PENDING expected");
        String errorMessage;
        if (job.getPhase() == ExecutionPhase.ERROR) {
            if (job.getErrorSummary() != null) {
                if (job.getErrorSummary().isHasDetail()) {
                    errorMessage = client.getErrorDetail(job.getJobId());
                } else {
                    errorMessage = job.getErrorSummary().getMessage();
                }
                if (job.getErrorSummary().getType() == ErrorType.TRANSIENT) {
                    if (!errorMessage.endsWith(".")) {
                        errorMessage += ".";
                    }
                    errorMessage += " Please retry later.";
                }
            } else {
                errorMessage = "Unexpected error";
            }
        } else {
            LOG.error("Error while executing job {}", job.getJobId());
            errorMessage = "Error while executing job. Job phase is "
                    + job.getPhase() + ". QUEUED, PENDING or EXECUTING expected";
        }
        throw new RuntimeException(errorMessage);
    }

    private String createTempListOfFilesNode(List<String> paths) {
+19 −0
Original line number Diff line number Diff line
@@ -90,6 +90,15 @@ public class VOSpaceClientTest {
        voSpaceClient.createNode(newNode);
    }
 
    @Test
    public void testGetErrorDetail() {

        CompletableFuture response = getMockedStringResponseFuture(200, "error message");
        when(mockedHttpClient.sendAsync(any(), any())).thenReturn(response);

        assertEquals("error message", voSpaceClient.getErrorDetail("123"));
    }

    protected static String getResourceFileContent(String fileName) {
        try ( InputStream in = VOSpaceClientTest.class.getClassLoader().getResourceAsStream(fileName)) {
            return new String(in.readAllBytes(), StandardCharsets.UTF_8);
@@ -102,6 +111,10 @@ public class VOSpaceClientTest {
        return CompletableFuture.completedFuture(getMockedStreamResponse(200, body));
    }

    protected static CompletableFuture<HttpResponse<String>> getMockedStringResponseFuture(int statusCode, String body) {
        return CompletableFuture.completedFuture(getMockedStringResponse(200, body));
    }

    protected static HttpResponse<InputStream> getMockedStreamResponse(int statusCode, String body) {
        HttpResponse response = getMockedResponse(statusCode);
        InputStream in = new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8));
@@ -109,6 +122,12 @@ public class VOSpaceClientTest {
        return response;
    }

    protected static HttpResponse<String> getMockedStringResponse(int statusCode, String body) {
        HttpResponse response = getMockedResponse(statusCode);
        when(response.body()).thenReturn(body);
        return response;
    }

    protected static HttpResponse getMockedResponse(int statusCode) {
        HttpResponse response = mock(HttpResponse.class);
        when(response.statusCode()).thenReturn(statusCode);
+67 −2
Original line number Diff line number Diff line
@@ -2,9 +2,14 @@ package it.inaf.ia2.vospace.ui.controller;

import it.inaf.ia2.vospace.ui.client.VOSpaceClient;
import java.util.Collections;
import java.util.function.Consumer;
import net.ivoa.xml.uws.v1.ErrorSummary;
import net.ivoa.xml.uws.v1.ErrorType;
import net.ivoa.xml.uws.v1.ExecutionPhase;
import net.ivoa.xml.uws.v1.JobSummary;
import net.ivoa.xml.vospace.v2.Protocol;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import org.junit.jupiter.api.Test;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
@@ -52,7 +57,7 @@ public class JobControllerTest {
    public void testMultipleFilesAsyncRecall() throws Exception {

        JobSummary job = new JobSummary();
        job.setPhase(ExecutionPhase.QUEUED);
        job.setPhase(ExecutionPhase.PENDING);

        when(client.startTransferJob(argThat(transfer -> {
            return transfer.getTarget().startsWith("vos://example.com!vospace/path/to/.tmp-");
@@ -68,4 +73,64 @@ public class JobControllerTest {
                .content("[\"/path/to/file1\", \"/path/to/file2\"]"))
                .andExpect(status().isOk());
    }

    @Test
    public void testAsyncRecallErrorWithDetails() throws Exception {

        JobSummary job = new JobSummary();
        job.setPhase(ExecutionPhase.ERROR);

        when(client.getErrorDetail(any())).thenReturn("Error was xxx");

        ErrorSummary errorSummary = new ErrorSummary();
        errorSummary.setHasDetail(true);
        errorSummary.setType(ErrorType.FATAL);

        job.setErrorSummary(errorSummary);

        when(client.startTransferJob(any())).thenReturn(job);

        testErrorCall(ex -> assertTrue(ex.getMessage().contains("Error was xxx")));
    }

    @Test
    public void testAsyncRecallTransientErrorWithoutDetails() throws Exception {

        JobSummary job = new JobSummary();
        job.setPhase(ExecutionPhase.ERROR);

        ErrorSummary errorSummary = new ErrorSummary();
        errorSummary.setMessage("simple message.");
        errorSummary.setType(ErrorType.TRANSIENT);

        job.setErrorSummary(errorSummary);

        when(client.startTransferJob(any())).thenReturn(job);

        testErrorCall(ex -> assertTrue(ex.getMessage().contains("simple message") && ex.getMessage().contains("retry")));
    }

    @Test
    public void testAsyncRecallErrorWithoutSummary() throws Exception {

        JobSummary job = new JobSummary();
        job.setPhase(ExecutionPhase.ERROR);

        when(client.startTransferJob(any())).thenReturn(job);

        testErrorCall(ex -> assertTrue(ex.getMessage().contains("error")));
    }

    private void testErrorCall(Consumer<Exception> exceptionChecker) throws Exception {
        try {
            mockMvc.perform(post("/recall")
                    .contentType(MediaType.APPLICATION_JSON)
                    .content("[\"/path/to/file\"]"))
                    .andExpect(status().is5xxServerError());
        } catch (Exception ex) {
            exceptionChecker.accept(ex);
            return;
        }
        fail("An error was expected");
    }
}