Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LogsResponse in AuditClient returns "ok: false" when request was a success #1373

Closed
Dunkhell opened this issue Sep 17, 2024 · 1 comment · Fixed by #1374
Closed

LogsResponse in AuditClient returns "ok: false" when request was a success #1373

Dunkhell opened this issue Sep 17, 2024 · 1 comment · Fixed by #1374
Assignees
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented project:slack-api-client project:slack-api-client
Milestone

Comments

@Dunkhell
Copy link

Dunkhell commented Sep 17, 2024

I'm using latest version of the SDK.

When requesting audit logs with any parameters the response is alwyas ok: false. The response doesn't provide warning or error message but does contain requested entries for the audit log.

The Slack SDK version

<dependency>
    <groupId>com.slack.api</groupId>
    <artifactId>slack-api-client</artifactId>
    <version>1.42.1</version>
</dependency>

Java Runtime version

openjdk version "17.0.8"

Steps to reproduce:

  1. Create AuditClient
  2. Create logs request with any parameters
  3. Check if "response.isOk()"

code ran:

public class AuditLogSlackService {

    private final Slack slack; // autowired using spring

    public List<LogsResponse.Entry> getLogs(String action, Integer latest, Integer oldest, String actor) {
        AuditClient auditClient = slack.audit("user-token");

        LogsRequest request = LogsRequest.builder()
                .token("user-token")
                .action(action)
                .build();

        Optional.ofNullable(latest).ifPresent(request::setLatest);
        Optional.ofNullable(oldest).ifPresent(request::setOldest);
        Optional.ofNullable(actor).ifPresent(request::setActor);

        RetryTemplate retry = RetryServiceUtil.getRetryTemplate(); // retry can be omitted for this example
        return retry.execute(arg0 -> {
            List<LogsResponse.Entry> result = new ArrayList<>();

            do {
                try {
                    LogsResponse response = auditClient.getLogs(request);
                    if (response.isOk()) {
                        result.addAll(response.getEntries());
                        request.setCursor(ofNullable(response.getResponseMetadata()).map(ResponseMetadata::getNextCursor).orElse(""));
                    } else {
                        throw new RuntimeException(response.getError());
                    }
                } catch (Exception e) {
                    log.info("{} Failed with {}", "LogsRequest", e.toString());
                    throw new RuntimeException(e.getMessage());
                }
                log.info("Retry finished for " + "LogsRequest");
            } while (!request.getCursor().isEmpty());
            return result;
        });
    }
}

Code contains custom exceptions and logging, which can be ignored since they do not result in the error

Expected result:

Response is ok: true and contains entries of the requested audit logs.

Actual result:

Response is ok: false and contains entries of the requested audit logs.

LogsResponse(rawBody=null, ok=false, warning=null, error=null, needed=null, provided=null, responseMetadata=ResponseMetadata(nextCursor=, messages=null, warnings=null), entries=[LogsResponse.Entry(all_entries_here....)])
@seratch seratch added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented project:slack-api-client project:slack-api-client and removed untriaged labels Sep 17, 2024
@seratch seratch added this to the 1.43.1 milestone Sep 17, 2024
@seratch seratch self-assigned this Sep 17, 2024
@seratch
Copy link
Member

seratch commented Sep 17, 2024

Hi @Dunkhell, thanks for reporting this issue. You're right that the property is not correctly set. We will resolve it in the next release, but while checking this, I found that actually the property is useless because when an error occurs, the API client throws an AuditApiException instead. So, we will make the property deprecated starting the next release. Please remove the code checking the property and handle AuditApiException properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented project:slack-api-client project:slack-api-client
Projects
None yet
2 participants