553869 - XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

In XPIProvider, when there is an error checking for updates, the onUpdateFinished event is fired with the error parameter set to constants defined in UpdateChecker. It should be using error codes defined in AddonManager, so that all providers can use them. Furthermore, consumers should not need to import UpdateChecker.jsm just to get the relevant error constants.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Does that also have a perf impact?

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

No perf impact - JSMs are cached, so importing it doesn't cost us anything, other than it being annoying. This is solely an API design issue.

Comment 3

[:Irving Reid (No longer working on Firefox)]

What's needed is:

1) make sure there are no inconsistencies between the error codes listed in UpdateChecker and those in AddonManager

2) make sure AddonManager has the definitive declarations for all the error codes

3) Require AddonManager.jsm in any other modules that refer to the error codes

4) Convert all uses of UpdateChecker.{error-code-name} to AddonManager.{error-code-name}

5) remove the declarations from UpdateChecker

It's possible there are add-ons that use the UpdateChecker.{error-code-name} declarations, so this may require add-on compatibility follow-up.

Comment 4

[Leni Kadali [:leni1] [GMT+3]]

Hello. Interested in working on this bug. Would like to know what files I would have to modify/create in order to submit a first patch. Thanks.

Comment 5

[Dave Townsend [:mossop]]

rhelmer is going to mentor this.

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #4)
> Hello. Interested in working on this bug. Would like to know what files I
> would have to modify/create in order to submit a first patch. Thanks.

Hi! I believe comment 3 is still a useful starting point.

Specifically you'd want to move the error codes from here:

http://searchfox.org/mozilla-central/rev/714606a8145636d93b116943d3a65a6a49d2acf8/toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm#784-796

Into this JSM instead: http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm

You'll need to modify any callers to use `AddonManager.{ERROR_CODE}` rather than `AddonUpdateChecker.{ERROR_CODE}`, you can either search in your local hg clone or search this way: http://searchfox.org/mozilla-central/search?q=symbol:%23ERROR_DOWNLOAD_ERROR&redirect=false

Thanks! Let me know if you need anything, happy to help.

Comment 7

[Leni Kadali [:leni1] [GMT+3]]

Attachment: XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Moved the error codes from AddonUpdateChecker.jsm to AddonManager.jsm
Modified callers to use `AddonManager.{ERROR_CODE}` rather than `AddonUpdateChecker.{ERROR_CODE}`
Hopefully if this passes review, I can push to MozReview

changed toolkit/mozapps/extensions/AddonManager.jsm
changed toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
changed toolkit/mozapps/extensions/test/browser/browser_updatessl.js

Comment 8

[Leni Kadali [:leni1] [GMT+3]]

Comment on attachment 8880298 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

I got this error submitting the patch:
abort: REST error on PUT to https://bugzilla.mozilla.org/rest/bug/553869: You tried to change the Assignee field from [e-mail] to [e-mail] , but only the assignee of the bug, or a user with the required permissions may change that field. If you are attempting to confirm an unconfirmed bug or edit the fields of a bug, find out how to get the necessary permissions.

If it's alright, may I be assigned the bug? :)

Comment 9

[Leni Kadali [:leni1] [GMT+3]]

Comment on attachment 8880298 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

I got this error submitting the patch:
abort: REST error on PUT to https://bugzilla.mozilla.org/rest/bug/553869: You tried to change the Assignee field from [e-mail] to [e-mail] , but only the assignee of the bug, or a user with the required permissions may change that field. If you are attempting to confirm an unconfirmed bug or edit the fields of a bug, find out how to get the necessary permissions.

If it's alright, may I be assigned the bug? :)

Comment 10

[Leni Kadali [:leni1] [GMT+3]]

> 5) remove the declarations from UpdateChecker
By the way, what does this bit mean? Does it mean I remove the `this.AddonUpdateChecker` code?

Comment 11

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #9)
> Comment on attachment 8880298 [details] [diff] [review]
> XPIProvider's onUpdateFinished uses error codes not defined in AddonManager
> 
> I got this error submitting the patch:
> abort: REST error on PUT to https://bugzilla.mozilla.org/rest/bug/553869:
> You tried to change the Assignee field from [e-mail] to
> [e-mail] , but only the assignee of the bug, or a user with
> the required permissions may change that field. If you are attempting to
> confirm an unconfirmed bug or edit the fields of a bug, find out how to get
> the necessary permissions.
> 
> If it's alright, may I be assigned the bug? :)

Ah yes it does mean that - I've assigned you now, sorry for not doing it before!

Comment 12

[Robert Helmer [:rhelmer]]

Comment on attachment 8880298 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Review of attachment 8880298 [details] [diff] [review]:
-----------------------------------------------------------------

Overall this looks good, needs a few changes before we can land it. Thanks for working on this!

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +3372,5 @@
>      // The addon did not have the expected ID
>      ["ERROR_INCORRECT_ID", -7],
>    ]),
>  
> +  // These must be kept in sync with AddonManager

This comment can be removed now.

@@ +3385,5 @@
> +  // The update information was not correctly signed or there was an SSL error.
> +  ERROR_SECURITY_ERROR: -5,
> +  // The update was cancelled
> +  ERROR_CANCELLED: -6,
> +  

Please remove this stray whitespace.

::: toolkit/mozapps/extensions/test/browser/browser_updatessl.js
@@ +2,5 @@
>   * http://creativecommons.org/publicdomain/zero/1.0/
>   */
>  
>  var tempScope = {};
> +Components.utils.import("resource://gre/modules/addons/AddonManager.jsm", tempScope);

`AddonUpdateChecker.jsm` still needs to be imported so please revert this line.

@@ +86,5 @@
>        message = "Should have seen the right result for an update check from " +
>                  mainURL;
>      }
>  
> +    AddonManager.checkForUpdates("[e-mail]",

This one is a function that is still in `AddonUpdateChecker` so please revert this line :)

Comment 13

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #10)
> > 5) remove the declarations from UpdateChecker
> By the way, what does this bit mean? Does it mean I remove the
> `this.AddonUpdateChecker` code?

The error codes such as `ERROR_TIMEOUT` and the comments (the bit that has been moved to `AddonManager`) should be removed, since they won't be used anymore.

Comment 14

[Leni Kadali [:leni1] [GMT+3]]

Attachment: XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Moved the error codes from AddonUpdateChecker.jsm to AddonManager.jsm
Modified callers to use `AddonManager.{ERROR_CODE}` rather than `AddonUpdateChecker.{ERROR_CODE}`
Removed the error codes e.g. `ERROR_TIMEOUT` and the corresponding comments from AddonUpdateChecker.jsm


changed toolkit/mozapps/extensions/AddonManager.jsm
changed toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
changed toolkit/mozapps/extensions/test/browser/browser_updatessl.js

Comment 15

[Leni Kadali [:leni1] [GMT+3]]

Comment on attachment 8880739 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

I wonder if I should revert this line 
`const DOWNLOAD_ERROR = AddonManager.ERROR_DOWNLOAD_ERROR;`
to `const DOWNLOAD_ERROR = AddonUpdateChecker.ERROR_DOWNLOAD_ERROR;`
or this isn't covered by the need of AddonUpdateChecker.jsm?

Comment 16

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #15)
> Comment on attachment 8880739 [details] [diff] [review]
> XPIProvider's onUpdateFinished uses error codes not defined in AddonManager
> 
> I wonder if I should revert this line 
> `const DOWNLOAD_ERROR = AddonManager.ERROR_DOWNLOAD_ERROR;`
> to `const DOWNLOAD_ERROR = AddonUpdateChecker.ERROR_DOWNLOAD_ERROR;`
> or this isn't covered by the need of AddonUpdateChecker.jsm?

I don't think this will work since you've now moved all these constants to AddonManager.

Have you tested this manually, or run any automated tests? I can help with that if not.

Comment 17

[Robert Helmer [:rhelmer]]

Comment on attachment 8880739 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Sorry for the delay! Was traveling and then attending a lot of meetings this week so haven't been at the computer much.

This looks good to me, I'd like to make sure this passes automated tests. I can go ahead and push this patch to our "try server" which will do this.

Comment 18

[Robert Helmer [:rhelmer]]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24e414f719ad5d3499352fc1725133b9b537c7ef

needinfo'ing myself so I don't forget to come back around and check.

Comment 19

[Leni Kadali [:leni1] [GMT+3]]

(In reply to Robert Helmer [:rhelmer] from comment #17)
> Comment on attachment 8880739 [details] [diff] [review]
> XPIProvider's onUpdateFinished uses error codes not defined in AddonManager
> 
> Sorry for the delay! Was traveling and then attending a lot of meetings this
> week so haven't been at the computer much.
> 
> This looks good to me, I'd like to make sure this passes automated tests. I
> can go ahead and push this patch to our "try server" which will do this.

I look forward to the results :). Thanks!

Comment 20

[Robert Helmer [:rhelmer]]

Comment on attachment 8880739 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Looks like the toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js test is failing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=24e414f719ad5d3499352fc1725133b9b537c7ef&selectedJob=111043879

Are you able to do a local build? You should be able to reproduce the failure with:

./mach test toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js

If not let me know and I can take a look. Thanks!

Comment 21

[Leni Kadali [:leni1] [GMT+3]]

(In reply to Robert Helmer [:rhelmer] from comment #20)
> Comment on attachment 8880739 [details] [diff] [review]
> XPIProvider's onUpdateFinished uses error codes not defined in AddonManager
> 
> Looks like the toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
> test is failing:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=24e414f719ad5d3499352fc1725133b9b537c7ef&selectedJob=1
> 11043879
> 
> Are you able to do a local build? You should be able to reproduce the
> failure with:
> 
> ./mach test toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
> 
> If not let me know and I can take a look. Thanks!

By local build, do you mean rebuilding Firefox. If that's the case, yes I have a local copy of the source code, so I can try and see.

Comment 22

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #21)
> (In reply to Robert Helmer [:rhelmer] from comment #20)
> > Comment on attachment 8880739 [details] [diff] [review]
> > XPIProvider's onUpdateFinished uses error codes not defined in AddonManager
> > 
> > Looks like the toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
> > test is failing:
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=24e414f719ad5d3499352fc1725133b9b537c7ef&selectedJob=1
> > 11043879
> > 
> > Are you able to do a local build? You should be able to reproduce the
> > failure with:
> > 
> > ./mach test toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
> > 
> > If not let me know and I can take a look. Thanks!
> 
> By local build, do you mean rebuilding Firefox. If that's the case, yes I
> have a local copy of the source code, so I can try and see.

Yes, that is correct.

Comment 23

[Leni Kadali [:leni1] [GMT+3]]

Ran the test and got the following output. 

0:10.77 TEST_STATUS: Thread-2  FAIL -3 == "undefined"
/home/user/src/mozilla-central/obj-x86_64-pc-linux-gnu/_tests/xpcshell/toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js:null:183
 0:10.77 LOG: Thread-2 INFO exiting test
 0:10.77 LOG: Thread-2 ERROR Unexpected exception 2147500036
undefined
 0:10.77 LOG: Thread-2 INFO exiting test
 0:10.79 LOG: Thread-2 INFO "CONSOLE_MESSAGE: (info) 1499453592748      addons.update-checker   WARN    onUpdateCheckComplete failed to parse update manifest: [Exception... "Missing updates property for urn:mozilla:extension:[e-mail]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/addons/AddonUpdateChecker.jsm :: parseRDFManifest :: line 369"  data: no] Stack trace: parseRDFManifest()@resource://gre/modules/addons/AddonUpdateChecker.jsm:369 < parser()@resource://gre/modules/addons/AddonUpdateChecker.jsm:645 < onLoad()@resource://gre/modules/addons/AddonUpdateChecker.jsm:655 < UpdateParser/<()@resource://gre/modules/addons/AddonUpdateChecker.jsm:580 < _do_main()@/home/user/src/mozilla-central/testing/xpcshell/head.js:222 < _execute_test()@/home/user/src/mozilla-central/testing/xpcshell/head.js:550 < -e:1"
 0:10.90 LOG: Thread-2 INFO <<<<<<<
 0:10.90 LOG: MainThread INFO INFO | Result summary:
 0:10.90 LOG: MainThread INFO INFO | Passed: 0
 0:10.90 LOG: MainThread INFO INFO | Failed: 2
 0:10.90 LOG: MainThread INFO INFO | Todo: 0
 0:10.90 LOG: MainThread INFO INFO | Retried: 0
 0:10.90 SUITE_END: MainThread 
Summary
=======

Ran 100 tests (2 parents, 98 subtests)
Expected results: 96
Unexpected results: 4 (FAIL: 4)

Unexpected Results
==================

xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
--------------------------------------------------------------------------------
FAIL [Parent]
xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
-------------------------------------------------------------------------
FAIL [Parent]
Tests were run in parallel. Try running with --sequential to make sure the failures were not caused by this.

What do you think is causing the error?

Comment 24

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #23)
> Ran the test and got the following output. 
> 
> 0:10.77 TEST_STATUS: Thread-2  FAIL -3 == "undefined"
> /home/user/src/mozilla-central/obj-x86_64-pc-linux-gnu/_tests/xpcshell/
> toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js:null:183
>  0:10.77 LOG: Thread-2 INFO exiting test
>  0:10.77 LOG: Thread-2 ERROR Unexpected exception 2147500036
> undefined
>  0:10.77 LOG: Thread-2 INFO exiting test
>  0:10.79 LOG: Thread-2 INFO "CONSOLE_MESSAGE: (info) 1499453592748     
> addons.update-checker   WARN    onUpdateCheckComplete failed to parse update
> manifest: [Exception... "Missing updates property for
> urn:mozilla:extension:[e-mail]"  nsresult:
> "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
> resource://gre/modules/addons/AddonUpdateChecker.jsm :: parseRDFManifest ::
> line 369"  data: no] Stack trace:
> parseRDFManifest()@resource://gre/modules/addons/AddonUpdateChecker.jsm:369
> < parser()@resource://gre/modules/addons/AddonUpdateChecker.jsm:645 <
> onLoad()@resource://gre/modules/addons/AddonUpdateChecker.jsm:655 <
> UpdateParser/<()@resource://gre/modules/addons/AddonUpdateChecker.jsm:580 <
> _do_main()@/home/user/src/mozilla-central/testing/xpcshell/head.js:222 <
> _execute_test()@/home/user/src/mozilla-central/testing/xpcshell/head.js:550
> < -e:1"
>  0:10.90 LOG: Thread-2 INFO <<<<<<<
>  0:10.90 LOG: MainThread INFO INFO | Result summary:
>  0:10.90 LOG: MainThread INFO INFO | Passed: 0
>  0:10.90 LOG: MainThread INFO INFO | Failed: 2
>  0:10.90 LOG: MainThread INFO INFO | Todo: 0
>  0:10.90 LOG: MainThread INFO INFO | Retried: 0
>  0:10.90 SUITE_END: MainThread 
> Summary
> =======
> 
> Ran 100 tests (2 parents, 98 subtests)
> Expected results: 96
> Unexpected results: 4 (FAIL: 4)
> 
> Unexpected Results
> ==================
> 
> xpcshell-unpack.ini:toolkit/mozapps/extensions/test/xpcshell/
> test_updatecheck.js
> -----------------------------------------------------------------------------
> ---
> FAIL [Parent]
> xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js
> -------------------------------------------------------------------------
> FAIL [Parent]
> Tests were run in parallel. Try running with --sequential to make sure the
> failures were not caused by this.
> 
> What do you think is causing the error?

If you take a look at test_updatecheck.js on line 183, it's still using `AddonUpdateChecker.ERROR_PARSE_ERROR` - this needs to be changed to `AddonManager` per your other changes. Hope that helps!

Comment 25

[Leni Kadali [:leni1] [GMT+3]]

Attachment: XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Moved the error codes from AddonUpdateChecker.jsm to AddonManager.jsm
Modified callers to use `AddonManager.{ERROR_CODE}` rather than `AddonUpdateChecker.{ERROR_CODE}`
Removed the error codes e.g. `ERROR_TIMEOUT` and the corresponding comments from AddonUpdateChecker.jsm
Updated test_updatecheck.js to use `AddonManager.ERROR_PARSE_ERROR` from `AddonUpdateChecker.ERROR_PARSE_ERROR`

changed toolkit/mozapps/extensions/AddonManager.jsm
changed toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
changed toolkit/mozapps/extensions/test/browser/browser_updatessl.js


changed toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js

Comment 26

[Leni Kadali [:leni1] [GMT+3]]

Comment on attachment 8886273 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Tests pass. Hopefully passes through on Try :-)

Comment 27

[Leni Kadali [:leni1] [GMT+3]]

Hi! If you're not a position to push to Try, could you guide me on how to do it? :-)

Comment 28

[Robert Helmer [:rhelmer]]

(In reply to Leni Kadali from comment #27)
> Hi! If you're not a position to push to Try, could you guide me on how to do
> it? :-)

I think you need "level 1" access to use try:
https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/

I went ahead and pushed this now, sorry for the delay (had some trouble with my try access, now resolved):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=812b9fc2bbd35b1ecab14e74bfb83cde96ae90f4

Comment 29

[Robert Helmer [:rhelmer]]

Comment on attachment 8886273 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Tests look good! There's just a tiny lint failure, you can test this locally with: `./mach lint toolkit/mozapps/extensions/AddonManager.jsm`

You can see this in the try run here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=812b9fc2bbd35b1ecab14e74bfb83cde96ae90f4&selectedJob=116748180

Comment 30

[Leni Kadali [:leni1] [GMT+3]]

Attachment: XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Moved the error codes from AddonUpdateChecker.jsm to AddonManager.jsm
Modified callers to use `AddonManager.{ERROR_CODE}` rather than `AddonUpdateChecker.{ERROR_CODE}`
Removed the error codes e.g. `ERROR_TIMEOUT` and the corresponding comments from AddonUpdateChecker.jsm
Updated test_updatecheck.js to use `AddonManager.ERROR_PARSE_ERROR` from `AddonUpdateChecker.ERROR_PARSE_ERROR`
Removed the whitespaces detected by `./mach lint toolkit/mozapps/extensions/AddonManager.jsm`
Should pass on Try.

changed toolkit/mozapps/extensions/AddonManager.jsm
changed toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm
changed toolkit/mozapps/extensions/test/browser/browser_updatessl.js
changed toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js

Comment 31

[Leni Kadali [:leni1] [GMT+3]]

Comment on attachment 8889855 [details] [diff] [review]
XPIProvider's onUpdateFinished uses error codes not defined in AddonManager

Should be good to go :-)

Comment 32

[Robert Helmer [:rhelmer]]

Now I am having another problem w/ running try locally (build system) - I am going to move this review over to mozreview so we can use the try integration and autoland etc.

Comment 33

[Robert Helmer [:rhelmer]]

Attachment: Bug 553869 - move error codes constants from UpdateChecker to AddonManager p=[e-mail]

Review commit: https://reviewboard.mozilla.org/r/162118/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162118/

Comment 34

[Robert Helmer [:rhelmer]]

Comment on attachment 8890910 [details]
Bug 553869 - move error codes constants from UpdateChecker to AddonManager  p=[e-mail]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162118/diff/1-2/

Comment 35

[Robert Helmer [:rhelmer]]

Comment on attachment 8890910 [details]
Bug 553869 - move error codes constants from UpdateChecker to AddonManager  p=[e-mail]

https://reviewboard.mozilla.org/r/162118/#review171272

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e13ecea8235
move error codes constants from UpdateChecker to AddonManager r=rhelmer p=[e-mail]

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3e13ecea8235

Comment 38

[Robert Helmer [:rhelmer]]

Looks like this landed successfully in mozilla-central and should be in a Nightly build soon.

Thanks for your contribution Leni Kadali!

Comment 39

[Caitlin Neiman [:caitmuenster]]

Thank you for the patch, Leni! \o/ Your contribution has been added to our recognition wiki: https://wiki.mozilla.org/Add-ons/Contribute/Recognition#August_2017

Welcome onboard! I hope to see you around. :)

Comment 40

[Leni Kadali [:leni1] [GMT+3]]

(In reply to Robert Helmer [:rhelmer] from comment #38)
> Looks like this landed successfully in mozilla-central and should be in a
> Nightly build soon.
> 
> Thanks for your contribution Leni Kadali!

You're most welcome Robert. And thanks for taking the time to mentor me :-)

(In reply to Caitlin Neiman (http://pronoun.is/she) from comment #39)
> Thank you for the patch, Leni! \o/ Your contribution has been added to our
> recognition wiki:
> https://wiki.mozilla.org/Add-ons/Contribute/Recognition#August_2017
> 
> Welcome onboard! I hope to see you around. :)

Thanks Caitlin! :D