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

Error status resulted at framework middleware handler is missing in auditInfo #839

Closed
jiachen1120 opened this issue Nov 24, 2020 · 4 comments
Assignees
Labels
bug Issue: Bug

Comments

@jiachen1120
Copy link
Contributor

Currently, Error status resulted at framework middleware handler is missing in auditInfo. The reason of it would be that the response sending may be completed before adding the status into auditInfo. Therefore, the ExchangeCompleteListener inside the audit handler may not be able to add the error status into the auditMap.

We may need to adjust te order of codes within LightHttpHandler.setExchangeStatus() method from

default void setExchangeStatus(HttpServerExchange exchange, Status status) {
        ...
        exchange.getResponseSender().send(status.toString()); 
        ...
        Map<String, Object> auditInfo = exchange.getAttachment(AttachmentConstants.AUDIT_INFO);
        if(auditInfo == null) {
            auditInfo = new HashMap<>();
            exchange.putAttachment(AttachmentConstants.AUDIT_INFO, auditInfo); 
        }

        // save info for auditing purposes in case of an error
        if(auditOnError)
            auditInfo.put(Constants.STATUS, status);
        if(auditStackTrace) {
            auditInfo.put(Constants.STACK_TRACE, Arrays.toString(elements));
        }
    }

to

default void setExchangeStatus(HttpServerExchange exchange, Status status) {
        ...
        Map<String, Object> auditInfo = exchange.getAttachment(AttachmentConstants.AUDIT_INFO);
        if(auditInfo == null) {
            auditInfo = new HashMap<>();
            exchange.putAttachment(AttachmentConstants.AUDIT_INFO, auditInfo); 
        }

        // save info for auditing purposes in case of an error
        if(auditOnError)
            auditInfo.put(Constants.STATUS, status);
        if(auditStackTrace) {
            auditInfo.put(Constants.STACK_TRACE, Arrays.toString(elements));
        }
        exchange.getResponseSender().send(status.toString()); 
    }

To send the response after setting error status.

@stevehu
Copy link
Contributor

stevehu commented Nov 24, 2020

@jiachen1120 Good catch!!! This makes perfect sense. Could submit a PR to get it fixed? Thanks.

@jiachen1120
Copy link
Contributor Author

@stevehu Sure. Will submit a PR!

jiachen1120 added a commit that referenced this issue Nov 25, 2020
… missing in auditInfo

- Added unit test
jiachen1120 added a commit that referenced this issue Nov 25, 2020
stevehu pushed a commit that referenced this issue Nov 30, 2020
#842)

* - fixed #839 Error status resulted at framework middleware handler is missing in auditInfo
- Added unit test

* - #839 revert unnecessary changes
@stevehu
Copy link
Contributor

stevehu commented Nov 30, 2020

@stevehu
Copy link
Contributor

stevehu commented Nov 30, 2020

younggwon1 pushed a commit to younggwon1/light-4j that referenced this issue Feb 10, 2024
…andler is… (networknt#842)

* - fixed networknt#839 Error status resulted at framework middleware handler is missing in auditInfo
- Added unit test

* - networknt#839 revert unnecessary changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue: Bug
Projects
None yet
Development

No branches or pull requests

3 participants