# We should never leak out exceptions in our controller tests

**URL:** https://meta.discourse.org/t/we-should-never-leak-out-exceptions-in-our-controller-tests/77800
**Category:** Development
**Created:** [January 12, 2018, 3:24am UTC](https://meta.discourse.org/t/we-should-never-leak-out-exceptions-in-our-controller-tests/77800 "2018-01-12T03:24:55Z")
**Posts on this page:** 1
**Page:** 1

<div class="post-metadata">

### Author: ![sam](https://sea3.discourse-cdn.com/meta/user_avatar/meta.discourse.org/sam/32/102149_2.png) [@sam](https://meta.discourse.org/u/sam)
#### Post date: [January 12, 2018, 3:24am UTC](https://meta.discourse.org/t/we-should-never-leak-out-exceptions-in-our-controller-tests/77800/1 "2018-01-12T03:24:55Z")

</div>

I just finished pushing a rather gnarly change per:

[https://github.com/discourse/discourse/commit/49ed382c2a7ddd11756e552e094319861418cd7f](https://github.com/discourse/discourse/commit/49ed382c2a7ddd11756e552e094319861418cd7f)

It basically replaces this pattern we used previously in tests:

```plaintext
 expect do
   get :index, format: :json
 end.to raise_error(Discourse::NotLoggedIn)

```

With this

```ruby
get :index, format: :json
expect(response.status).to eq(403)

```

I made this change so our `test` environment is **much more inline** with our `production` environment. Previously we were leaking out a few exceptions and erroneously returning 500 status when we had a far more appropriate status to return.

From an API standpoint we never ever want exceptions to leak out, what we do want is to ensure correct error codes are returned to clients and that we don’t mutate data we should not for users that are not allowed access.

This change ensures that we stay “honest” and treat our tests just like we treat production.

So now our test environment has ` config.action_dispatch.show_exceptions = true` just like we have in production.

And our test environment has `config.consider_all_requests_local = false` just like we have in production.

This will help catch future API inconsistencies going forward and various edge cases.

* * *

Note:

We still have this nasty in our specs

```plaintext
it 'requires a username_or_email parameter' do
   expect { put :create, format: :json }.to raise_error(ActionController::ParameterMissing)
end

```

This happens cause we never bother rescuing from `ActionController::ParameterMissing` and are erroneously returning 500s here (which are generated in the safeguard in unicorn), I went with a 400 for InvalidParams and should rescue this as well.