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

Added "onTaffyRequestEnd" functionality #279

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@donwalter
Contributor

donwalter commented Jul 7, 2015

Added "onTaffyRequestEnd" functionality, has all the same arguments as "onTaffyRequest" and also adds the response, both in the original format and the parsed response in the format the user has requested. Can be used for global logging or other troubleshooting

Tested with

  • ColdFusion 10,291717
  • Java 1.7
  • Taffy version 3.0.2

Suggested Documentation (Feel free to edit as needed)

onTaffyRequestEnd()

Parameters:

  • verb (string) - The HTTP request verb provided by the client
  • cfc (string) - The CFC name (minus ".cfc") that would handle the request. (Bean Name, if using an external bean factory.)
  • requestArguments (struct) - A structure containing all of the arguments of the request, including tokens from the URI as well as any query string parameters.
  • mimeExt (string) - The mime extension (e.g. "json" - NOT the full mime type, e.g. "application/json")
  • headers (struct) - A structure containing each header from the request, as sent by the client.
  • methodMetadata (struct) - A structure containing any non-taffy metadata set on the requested resource method.
  • matchedURI (string) - The taffy:uri value, including unreplaced tokens, from the CFC that matches the request. (e.g. /foo/{bar})
  • parsedResponse (string) - The response, parsed in the format that the user has requested
  • originalResponse (struct/array/query) - The response in the original format, before being converted to JSON/XML

This method is optional, and allows you to add global functionality to occur after the call is complete. If you implement it, you can do things such as global logging for all calls.

This function should always return TRUE

Don Walter
Added "onTaffyRequestEnd" functionality
Added "onTaffyRequestEnd" functionality, has all the same arguments as "onTaffyRequest" and also adds the response, both in the original format and the parsed response in the format the user has requested. Can be used for global logging or other troubleshooting
@admentus

This comment has been minimized.

Show comment
Hide comment
@admentus

admentus Jul 10, 2015

Ran into a problem with this patch ... somehow it is breaking the CORS config.

If I comment out the block in api.cfc (lines 477-487) where the onTaffyRequestEnd method is being called, everything works fine. I removed my custom implementation of that method in my Application.cfc just to make sure it was not something I had done and the issue persists.

Only after I disable the call to the method all together does the CORS error go away.

I am not an expert in CORS by any means, so I am still digging here, but the errors shown in the console are as follows:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://myserver.dev/index.cfm/session. (Reason: CORS preflight channel did not succeed).

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://myserver.dev/index.cfm/session. (Reason: CORS request failed).

admentus commented Jul 10, 2015

Ran into a problem with this patch ... somehow it is breaking the CORS config.

If I comment out the block in api.cfc (lines 477-487) where the onTaffyRequestEnd method is being called, everything works fine. I removed my custom implementation of that method in my Application.cfc just to make sure it was not something I had done and the issue persists.

Only after I disable the call to the method all together does the CORS error go away.

I am not an expert in CORS by any means, so I am still digging here, but the errors shown in the console are as follows:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://myserver.dev/index.cfm/session. (Reason: CORS preflight channel did not succeed).

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://myserver.dev/index.cfm/session. (Reason: CORS request failed).

@admentus

This comment has been minimized.

Show comment
Hide comment
@admentus

admentus Jul 13, 2015

Ok, I finally tracked this down with Dan Skaggs help. The issue is in the last two arguments being passed to the onTaffyRequestEnd method. With an OPTIONS verb, there is no result, so this throws a 500 error which results in the CORS error on the browser side (hiding the actual error).

I have not gone to look for the backwards compatibility matrix for Taffy, but these two arguments need to be checked for existence before being passed in like this ...

    <cfset _taffyRequest.continue = onTaffyRequestEnd(
        _taffyRequest.verb
        ,_taffyRequest.matchDetails.beanName
        ,_taffyRequest.requestArguments
        ,_taffyRequest.returnMimeExt
        ,_taffyRequest.headers
        ,_taffyRequest.methodMetadata
        ,local.parsed.matchDetails.srcUri
        ,structKeyExists( _taffyRequest, 'resultSerialized' ) ? _taffyRequest.resultSerialized : ''
        ,structKeyExists( _taffyRequest, 'result' ) ? _taffyRequest.result.getData() : {}
    ) />

With that, I believe everything is good.

admentus commented Jul 13, 2015

Ok, I finally tracked this down with Dan Skaggs help. The issue is in the last two arguments being passed to the onTaffyRequestEnd method. With an OPTIONS verb, there is no result, so this throws a 500 error which results in the CORS error on the browser side (hiding the actual error).

I have not gone to look for the backwards compatibility matrix for Taffy, but these two arguments need to be checked for existence before being passed in like this ...

    <cfset _taffyRequest.continue = onTaffyRequestEnd(
        _taffyRequest.verb
        ,_taffyRequest.matchDetails.beanName
        ,_taffyRequest.requestArguments
        ,_taffyRequest.returnMimeExt
        ,_taffyRequest.headers
        ,_taffyRequest.methodMetadata
        ,local.parsed.matchDetails.srcUri
        ,structKeyExists( _taffyRequest, 'resultSerialized' ) ? _taffyRequest.resultSerialized : ''
        ,structKeyExists( _taffyRequest, 'result' ) ? _taffyRequest.result.getData() : {}
    ) />

With that, I believe everything is good.

@atuttle

This comment has been minimized.

Show comment
Hide comment
@atuttle

atuttle Jul 13, 2015

Owner

Nicely done! CORS errors can be so difficult to track down sometimes.

Owner

atuttle commented Jul 13, 2015

Nicely done! CORS errors can be so difficult to track down sometimes.

@atuttle

This comment has been minimized.

Show comment
Hide comment
@atuttle

atuttle Sep 28, 2015

Owner

Thanks for the PR @donwalter, and the bugfix @admentus! I'm making two small additional changes:

  • We weren't reporting the time-spent in the OTRE handler function, so I'm adding a header for that
  • No need to require the function to return true or check its result at all, so I'm removing the check.
Owner

atuttle commented Sep 28, 2015

Thanks for the PR @donwalter, and the bugfix @admentus! I'm making two small additional changes:

  • We weren't reporting the time-spent in the OTRE handler function, so I'm adding a header for that
  • No need to require the function to return true or check its result at all, so I'm removing the check.

atuttle added a commit that referenced this pull request Sep 28, 2015

@atuttle

This comment has been minimized.

Show comment
Hide comment
@atuttle

atuttle Sep 28, 2015

Owner

Whatever I did confused GitHub -- not to worry, your PR has been merged!

Owner

atuttle commented Sep 28, 2015

Whatever I did confused GitHub -- not to worry, your PR has been merged!

@atuttle atuttle closed this Sep 28, 2015

donwalter pushed a commit to donwalter/Taffy that referenced this pull request Nov 15, 2016

Don Walter
Fix for Issue 323 (Headers regression from #279)
atuttle#323

Changed outputs to set variables and then output after all headers have
been sent

atuttle added a commit that referenced this pull request Nov 15, 2016

Merge pull request #330 from donwalter/master
Fix for Issue 323 (Headers regression from #279)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment