Allows for a block of code to always run after the endpoint #1864

myxoh · 2019-02-20T19:54:05Z

Motive: Sometime we do things like logging, or open sessions, or other actions during an API request and we want to make sure those actions are wrapped up afterwards.

This introduces a DSL way of handling this after every request on an API

lib/grape/dsl/request_response.rb

lib/grape/middleware/error.rb

LeFnord · 2019-02-21T16:05:26Z

is it a different funcionality as this one #before-and-after?

myxoh · 2019-02-21T16:31:46Z

@LeFnord
It's related to the before-and-after (potentially that section would be a better place on the description)
The difference between after and ensure_block is that ensure_block will run even when there has been an exception on the API while after will not

LeFnord · 2019-02-21T17:28:06Z

… ok, something like an around 😉

myxoh · 2019-02-21T17:33:04Z

Difference would be that this is not run before.
So the cycle would be
before
api_call
IF success
after
END
ensure

LeFnord · 2019-02-21T17:41:30Z

nice … please update above readme section with this, thanks

dm1try · 2019-02-21T23:51:35Z

is it a different funcionality as this one #before-and-after?
It's related to the before-and-after

I don't think so; the implementation looks like a missing complementary part for rescue_from sugar and even lives in the error middleware.

Sometime we do things like logging, or open sessions, or other actions during an API request and we want to make sure those actions are wrapped up afterwards.

maybe this can be solved by using an endpoint middleware(the spec)

myxoh · 2019-02-22T12:44:32Z

I've updated the lifecycle clarifying how this will run. I agree that it could be sorted by Middleware @dm1try but I do feel like having an after block that always run should be a feature in Grape regardless.
Could we move the discussion around the feature to the issue: #1865 and discuss over here only the implementation?

dm1try · 2019-02-23T15:50:21Z

I've updated the lifecycle clarifying how this will run.

Users might be confused by the current implementation, as the description hints that ensure_block is the part of a request lifecycle, though this block is run in the different scope(in the error middleware) which means it does not share the endpoint state.

before do
 @state = true
end
ensure_block do
  @state # error
end

Moreover according to the ordering in the current middleware stack, ensure_block can be called without a preceding before block if the error will be raised before an endpoint run(for example, versioner middleware can raise an error that will be caught by error middleware); it feels unexpected.

but I do feel like having an after block that always run should be a feature in Grape regardless.

btw, the same behavior that proposed here can be emulated by using filters middleware without providing anything new:

use Grape::Middleware::Filter, after: -> { do_something }

dblock · 2019-02-24T20:29:14Z

I like the syntactic sugar of ensure_block. I would prefer ensure, but I presume it won't work as a reserved keyword? naming is hard :)

I think @dm1try has a very valid point (nice catch) about scope of execution and corresponding before, I think that needs to be fixed and explained clearly what the scope of execution is

myxoh · 2019-02-24T20:38:41Z

Nice catch @dm1try, I'll give it a go at moving the scope, this was just a simple MVP.

Regarding wording, I went for ensure initially but yes, it being a reserved keyword made things nasty.

We could go with things like always or finally (borrowing from other languages)

dblock · 2019-02-24T23:15:43Z

How do others feel about finally vs. ensure_block?

myxoh · 2019-02-25T14:31:49Z

after some thought I think I like finally better. I've also proceeded to move the logic to the endpoint so it shares scope with other callbacks like before and the helpers.

LeFnord · 2019-02-25T16:00:05Z

+1 for finally

dblock

Looks great. Only some nitpicky comments around my English language OCD

dblock · 2019-02-25T16:19:41Z

README.md


 Blocks can be executed before or after every API call, using `before`, `after`,
 `before_validation` and `after_validation`.
+If the API fails the `after` call will not be trigered, if you need code to execute for sure
+use the `finally`


Missing period.

dblock · 2019-02-25T16:20:51Z

README.md

+4. `after_validation` (if Validation Successful)
+5. _the API call_ (if Validation Successful)
+6. `after` (if Validation & API Successful)
+7. `finally` (ALWAYS)


Nitpicking, let's not SHOUT :) I would make all of the comments lowercase (upon successful validation, always, etc.)

dblock · 2019-02-25T16:21:03Z

README.md

@@ -3121,6 +3124,13 @@ before do
 end
 ```

+You can ensure a block of code runs after every request (including failures) with `finally`:


Add a space below.

dblock · 2019-02-25T16:22:04Z

README.md

+You can ensure a block of code runs after every request (including failures) with `finally`:
+```ruby
+finally do
+  This.block(of: code) # Will definetely run after every request


maybe just # this code will run after every successful or failed request, cause This.block(of: code) is not ruby

dblock · 2019-02-25T16:23:04Z

lib/grape/dsl/callbacks.rb

+
+        # Allows you to specify a something that will always be executed after a call
+        # API call. Unlike the `after` block, this code will run even on
+        # unsuccesful requests


Add a period after requests.

dblock · 2019-02-25T16:23:36Z

lib/grape/endpoint.rb


-        # status verifies body presence when DELETE
-        @body ||= response_object
+          # The Body commonly is an Array of Strings, the application instance itself, or a File-like object


lowercase body

(I'm merely moving this around but happy to address this anyway)

dblock · 2019-02-25T16:24:38Z

I'm cool with this change if @dmitry is cool with this change?

lib/grape/endpoint.rb

myxoh · 2019-02-25T18:38:59Z

I'll address your changes @dblock as soon as @dm1try helps me decide on the situation of a finally raising an error

myxoh · 2019-02-26T12:09:52Z

Addressed

dblock · 2019-02-26T20:10:04Z

Merged.

I want to thank @dm1try @LeFnord and of course @myxoh for this beautiful example of collaboration, strong, but not stubborn opinions and an excellent feature that will make API developers' work easier. 🍻

…pe#1864) Closes ruby-grape#1865

myxoh added 2 commits February 20, 2019 19:48

[Ensurer] Adds a way of ensuring a code block run

7c86e92

[Ensurer] Documents ensure_block and renames to ensure_block

948ef5d

myxoh added the feature request label Feb 20, 2019

myxoh mentioned this pull request Feb 20, 2019

Ensure code block after every request - finally block #1865

Closed

myxoh added 2 commits February 20, 2019 19:56

Adds PR to the changelog

439932a

Fixes readme

8df7235

undersky0 approved these changes Feb 21, 2019

View reviewed changes

lib/grape/dsl/request_response.rb Outdated Show resolved Hide resolved

lib/grape/middleware/error.rb Outdated Show resolved Hide resolved

myxoh added 2 commits February 22, 2019 12:33

Adds readme following lifecycle and validates lifecycle through testitng

2c622ed

Documents the behaviour of ensure_block

20260c4

myxoh added 3 commits February 25, 2019 14:19

Moves ensure_block to callbacks and endpoint

338a845

Renames ensure block to finally

cbdb07d

Adds finally to the notifications spec in the endpoints spec

111f714

dblock approved these changes Feb 25, 2019

View reviewed changes

dm1try reviewed Feb 25, 2019

View reviewed changes

lib/grape/endpoint.rb Outdated Show resolved Hide resolved

Removes unnecesary begin / end

41b7965

Addresses comments and documentation syntax / spelling / wording

e505e3d

dm1try approved these changes Feb 26, 2019

View reviewed changes

dblock merged commit b79bd9c into ruby-grape:master Feb 26, 2019

basjanssen pushed a commit to basjanssen/grape that referenced this pull request Feb 28, 2020

Allows for a block of code to always run after the endpoint (ruby-gra…

8b83164

…pe#1864) Closes ruby-grape#1865

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allows for a block of code to always run after the endpoint #1864

Allows for a block of code to always run after the endpoint #1864

myxoh commented Feb 20, 2019 •

edited

LeFnord commented Feb 21, 2019

myxoh commented Feb 21, 2019

LeFnord commented Feb 21, 2019

myxoh commented Feb 21, 2019 •

edited

LeFnord commented Feb 21, 2019

dm1try commented Feb 21, 2019

myxoh commented Feb 22, 2019

dm1try commented Feb 23, 2019 •

edited

dblock commented Feb 24, 2019

myxoh commented Feb 24, 2019

dblock commented Feb 24, 2019

myxoh commented Feb 25, 2019

LeFnord commented Feb 25, 2019

dblock left a comment

dblock Feb 25, 2019

dblock Feb 25, 2019

dblock Feb 25, 2019

dblock Feb 25, 2019

dblock Feb 25, 2019

dblock Feb 25, 2019

myxoh Feb 25, 2019

dblock commented Feb 25, 2019

myxoh commented Feb 25, 2019

myxoh commented Feb 26, 2019

dblock commented Feb 26, 2019

Allows for a block of code to always run after the endpoint #1864

Allows for a block of code to always run after the endpoint #1864

Conversation

myxoh commented Feb 20, 2019 • edited

LeFnord commented Feb 21, 2019

myxoh commented Feb 21, 2019

LeFnord commented Feb 21, 2019

myxoh commented Feb 21, 2019 • edited

LeFnord commented Feb 21, 2019

dm1try commented Feb 21, 2019

myxoh commented Feb 22, 2019

dm1try commented Feb 23, 2019 • edited

dblock commented Feb 24, 2019

myxoh commented Feb 24, 2019

dblock commented Feb 24, 2019

myxoh commented Feb 25, 2019

LeFnord commented Feb 25, 2019

dblock left a comment

Choose a reason for hiding this comment

dblock Feb 25, 2019

Choose a reason for hiding this comment

dblock Feb 25, 2019

Choose a reason for hiding this comment

dblock Feb 25, 2019

Choose a reason for hiding this comment

dblock Feb 25, 2019

Choose a reason for hiding this comment

dblock Feb 25, 2019

Choose a reason for hiding this comment

dblock Feb 25, 2019

Choose a reason for hiding this comment

myxoh Feb 25, 2019

Choose a reason for hiding this comment

dblock commented Feb 25, 2019

myxoh commented Feb 25, 2019

myxoh commented Feb 26, 2019

dblock commented Feb 26, 2019

myxoh commented Feb 20, 2019 •

edited

myxoh commented Feb 21, 2019 •

edited

dm1try commented Feb 23, 2019 •

edited