diff options
45 files changed, 967 insertions, 40 deletions
diff --git a/app/assets/javascripts/error_tracking/components/error_details.vue b/app/assets/javascripts/error_tracking/components/error_details.vue new file mode 100644 index 00000000000..37c9818f869 --- /dev/null +++ b/app/assets/javascripts/error_tracking/components/error_details.vue @@ -0,0 +1,141 @@ +<script> +import { mapActions, mapGetters, mapState } from 'vuex'; +import dateFormat from 'dateformat'; +import { __, sprintf } from '~/locale'; +import { GlButton, GlLink, GlLoadingIcon } from '@gitlab/ui'; +import Icon from '~/vue_shared/components/icon.vue'; +import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; +import Stacktrace from './stacktrace.vue'; +import TrackEventDirective from '~/vue_shared/directives/track_event'; +import timeagoMixin from '~/vue_shared/mixins/timeago'; +import { trackClickErrorLinkToSentryOptions } from '../utils'; + +export default { + components: { + GlButton, + GlLink, + GlLoadingIcon, + TooltipOnTruncate, + Icon, + Stacktrace, + }, + directives: { + TrackEvent: TrackEventDirective, + }, + mixins: [timeagoMixin], + props: { + issueDetailsPath: { + type: String, + required: true, + }, + issueStackTracePath: { + type: String, + required: true, + }, + }, + computed: { + ...mapState('details', ['error', 'loading', 'loadingStacktrace', 'stacktraceData']), + ...mapGetters('details', ['stacktrace']), + reported() { + return sprintf( + __('Reported %{timeAgo} by %{reportedBy}'), + { + reportedBy: `<strong>${this.error.culprit}</strong>`, + timeAgo: this.timeFormated(this.stacktraceData.date_received), + }, + false, + ); + }, + firstReleaseLink() { + return `${this.error.external_base_url}/releases/${this.error.first_release_short_version}`; + }, + lastReleaseLink() { + return `${this.error.external_base_url}releases/${this.error.last_release_short_version}`; + }, + showDetails() { + return Boolean(!this.loading && this.error && this.error.id); + }, + showStacktrace() { + return Boolean(!this.loadingStacktrace && this.stacktrace && this.stacktrace.length); + }, + }, + mounted() { + this.startPollingDetails(this.issueDetailsPath); + this.startPollingStacktrace(this.issueStackTracePath); + }, + methods: { + ...mapActions('details', ['startPollingDetails', 'startPollingStacktrace']), + trackClickErrorLinkToSentryOptions, + formatDate(date) { + return `${this.timeFormated(date)} (${dateFormat(date, 'UTC:yyyy-mm-dd h:MM:ssTT Z')})`; + }, + }, +}; +</script> + +<template> + <div> + <div v-if="loading" class="py-3"> + <gl-loading-icon :size="3" /> + </div> + + <div v-else-if="showDetails" class="error-details"> + <div class="top-area align-items-center justify-content-between py-3"> + <span v-if="!loadingStacktrace && stacktrace" v-html="reported"></span> + <!-- <gl-button class="my-3 ml-auto" variant="success"> + {{ __('Create Issue') }} + </gl-button>--> + </div> + <div> + <tooltip-on-truncate :title="error.title" truncate-target="child" placement="top"> + <h2 class="text-truncate">{{ error.title }}</h2> + </tooltip-on-truncate> + <h3>{{ __('Error details') }}</h3> + <ul> + <li> + <span class="bold">{{ __('Sentry event') }}:</span> + <gl-link + v-track-event="trackClickErrorLinkToSentryOptions(error.external_url)" + :href="error.external_url" + target="_blank" + > + <span class="text-truncate">{{ error.external_url }}</span> + <icon name="external-link" class="ml-1 flex-shrink-0" /> + </gl-link> + </li> + <li v-if="error.first_release_short_version"> + <span class="bold">{{ __('First seen') }}:</span> + {{ formatDate(error.first_seen) }} + <gl-link :href="firstReleaseLink" target="_blank"> + <span>{{ __('Release') }}: {{ error.first_release_short_version }}</span> + </gl-link> + </li> + <li v-if="error.last_release_short_version"> + <span class="bold">{{ __('Last seen') }}:</span> + {{ formatDate(error.last_seen) }} + <gl-link :href="lastReleaseLink" target="_blank"> + <span>{{ __('Release') }}: {{ error.last_release_short_version }}</span> + </gl-link> + </li> + <li> + <span class="bold">{{ __('Events') }}:</span> + <span>{{ error.count }}</span> + </li> + <li> + <span class="bold">{{ __('Users') }}:</span> + <span>{{ error.user_count }}</span> + </li> + </ul> + + <div v-if="loadingStacktrace" class="py-3"> + <gl-loading-icon :size="3" /> + </div> + + <template v-if="showStacktrace"> + <h3 class="my-4">{{ __('Stack trace') }}</h3> + <stacktrace :entries="stacktrace" /> + </template> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index a76eb747c34..88139ce7403 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -8,11 +8,12 @@ import { GlTable, GlSearchBoxByType, } from '@gitlab/ui'; +import { visitUrl } from '~/lib/utils/url_utility'; import Icon from '~/vue_shared/components/icon.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import { __ } from '~/locale'; import TrackEventDirective from '~/vue_shared/directives/track_event'; -import { trackViewInSentryOptions, trackClickErrorLinkToSentryOptions } from '../utils'; +import { trackViewInSentryOptions } from '../utils'; export default { fields: [ @@ -62,8 +63,8 @@ export default { }; }, computed: { - ...mapState(['errors', 'externalUrl', 'loading']), - ...mapGetters(['filterErrorsByTitle']), + ...mapState('list', ['errors', 'externalUrl', 'loading']), + ...mapGetters('list', ['filterErrorsByTitle']), filteredErrors() { return this.errorSearchQuery ? this.filterErrorsByTitle(this.errorSearchQuery) : this.errors; }, @@ -74,9 +75,11 @@ export default { } }, methods: { - ...mapActions(['startPolling', 'restartPolling']), + ...mapActions('list', ['startPolling', 'restartPolling']), trackViewInSentryOptions, - trackClickErrorLinkToSentryOptions, + viewDetails(errorId) { + visitUrl(`error_tracking/${errorId}/details`); + }, }, }; </script> @@ -125,13 +128,11 @@ export default { <template slot="error" slot-scope="errors"> <div class="d-flex flex-column"> <gl-link - v-track-event="trackClickErrorLinkToSentryOptions(errors.item.externalUrl)" - :href="errors.item.externalUrl" class="d-flex text-dark" target="_blank" + @click="viewDetails(errors.item.id)" > <strong class="text-truncate">{{ errors.item.title.trim() }}</strong> - <icon name="external-link" class="ml-1 flex-shrink-0" /> </gl-link> <span class="text-secondary text-truncate"> {{ errors.item.culprit }} diff --git a/app/assets/javascripts/error_tracking/components/stacktrace.vue b/app/assets/javascripts/error_tracking/components/stacktrace.vue new file mode 100644 index 00000000000..6b71967624f --- /dev/null +++ b/app/assets/javascripts/error_tracking/components/stacktrace.vue @@ -0,0 +1,33 @@ +<script> +import StackTraceEntry from './stacktrace_entry.vue'; + +export default { + components: { + StackTraceEntry, + }, + props: { + entries: { + type: Array, + required: true, + }, + }, + methods: { + isFirstEntry(index) { + return index === 0; + }, + }, +}; +</script> + +<template> + <div class="stacktrace"> + <stack-trace-entry + v-for="(entry, index) in entries" + :key="`stacktrace-entry-${index}`" + :lines="entry.context" + :file-path="entry.filename" + :error-line="entry.lineNo" + :expanded="isFirstEntry(index)" + /> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue b/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue new file mode 100644 index 00000000000..ad542c579a9 --- /dev/null +++ b/app/assets/javascripts/error_tracking/components/stacktrace_entry.vue @@ -0,0 +1,110 @@ +<script> +import { GlTooltip } from '@gitlab/ui'; +import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; +import Icon from '~/vue_shared/components/icon.vue'; + +export default { + components: { + ClipboardButton, + FileIcon, + Icon, + }, + directives: { + GlTooltip, + }, + props: { + lines: { + type: Array, + required: true, + }, + filePath: { + type: String, + required: true, + }, + errorLine: { + type: Number, + required: true, + }, + expanded: { + type: Boolean, + required: false, + default: false, + }, + }, + data() { + return { + isExpanded: this.expanded, + }; + }, + computed: { + linesLength() { + return this.lines.length; + }, + collapseIcon() { + return this.isExpanded ? 'chevron-down' : 'chevron-right'; + }, + }, + methods: { + isHighlighted(lineNum) { + return lineNum === this.errorLine; + }, + toggle() { + this.isExpanded = !this.isExpanded; + }, + lineNum(line) { + return line[0]; + }, + lineCode(line) { + return line[1]; + }, + }, + userColorScheme: window.gon.user_color_scheme, +}; +</script> + +<template> + <div class="file-holder"> + <div ref="header" class="file-title file-title-flex-parent"> + <div class="file-header-content "> + <div class="d-inline-block cursor-pointer" @click="toggle()"> + <icon :name="collapseIcon" :size="16" aria-hidden="true" class="append-right-5" /> + </div> + <div class="d-inline-block append-right-4"> + <file-icon + :file-name="filePath" + :size="18" + aria-hidden="true" + css-classes="append-right-5" + /> + <strong v-gl-tooltip :title="filePath" class="file-title-name" data-container="body"> + {{ filePath }} + </strong> + </div> + + <clipboard-button + :title="__('Copy file path')" + :text="filePath" + css-class="btn-default btn-transparent btn-clipboard" + /> + </div> + </div> + + <table v-if="isExpanded" :class="$options.userColorScheme" class="code js-syntax-highlight"> + <tbody> + <template v-for="(line, index) in lines"> + <tr :key="`stacktrace-line-${index}`" class="line_holder"> + <td class="diff-line-num" :class="{ old: isHighlighted(lineNum(line)) }"> + {{ lineNum(line) }} + </td> + <td + class="line_content" + :class="{ old: isHighlighted(lineNum(line)) }" + v-html="lineCode(line)" + ></td> + </tr> + </template> + </tbody> + </table> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking/details.js b/app/assets/javascripts/error_tracking/details.js new file mode 100644 index 00000000000..b9b51a6539f --- /dev/null +++ b/app/assets/javascripts/error_tracking/details.js @@ -0,0 +1,25 @@ +import Vue from 'vue'; +import store from './store'; +import ErrorDetails from './components/error_details.vue'; + +export default () => { + // eslint-disable-next-line no-new + new Vue({ + el: '#js-error_details', + components: { + ErrorDetails, + }, + store, + render(createElement) { + const domEl = document.querySelector(this.$options.el); + const { issueDetailsPath, issueStackTracePath } = domEl.dataset; + + return createElement('error-details', { + props: { + issueDetailsPath, + issueStackTracePath, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/error_tracking/index.js b/app/assets/javascripts/error_tracking/list.js index 073e2c8f1c7..073e2c8f1c7 100644 --- a/app/assets/javascripts/error_tracking/index.js +++ b/app/assets/javascripts/error_tracking/list.js diff --git a/app/assets/javascripts/error_tracking/services/index.js b/app/assets/javascripts/error_tracking/services/index.js index ab89521dc46..68988296cc2 100644 --- a/app/assets/javascripts/error_tracking/services/index.js +++ b/app/assets/javascripts/error_tracking/services/index.js @@ -1,7 +1,7 @@ import axios from '~/lib/utils/axios_utils'; export default { - getErrorList({ endpoint }) { + getSentryData({ endpoint }) { return axios.get(endpoint); }, }; diff --git a/app/assets/javascripts/error_tracking/store/details/actions.js b/app/assets/javascripts/error_tracking/store/details/actions.js new file mode 100644 index 00000000000..0390bca7175 --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/details/actions.js @@ -0,0 +1,63 @@ +import service from '../../services'; +import * as types from './mutation_types'; +import createFlash from '~/flash'; +import Poll from '~/lib/utils/poll'; +import { __ } from '~/locale'; + +let stackTracePoll; +let detailPoll; + +const stopPolling = poll => { + if (poll) poll.stop(); +}; + +export function startPollingDetails({ commit }, endpoint) { + detailPoll = new Poll({ + resource: service, + method: 'getSentryData', + data: { endpoint }, + successCallback: ({ data }) => { + if (!data) { + detailPoll.restart(); + return; + } + + commit(types.SET_ERROR, data.error); + commit(types.SET_LOADING, false); + + stopPolling(detailPoll); + }, + errorCallback: () => { + commit(types.SET_LOADING, false); + createFlash(__('Failed to load error details from Sentry.')); + }, + }); + + detailPoll.makeRequest(); +} + +export function startPollingStacktrace({ commit }, endpoint) { + stackTracePoll = new Poll({ + resource: service, + method: 'getSentryData', + data: { endpoint }, + successCallback: ({ data }) => { + if (!data) { + stackTracePoll.restart(); + return; + } + commit(types.SET_STACKTRACE_DATA, data.error); + commit(types.SET_LOADING_STACKTRACE, false); + + stopPolling(stackTracePoll); + }, + errorCallback: () => { + commit(types.SET_LOADING_STACKTRACE, false); + createFlash(__('Failed to load stacktrace.')); + }, + }); + + stackTracePoll.makeRequest(); +} + +export default () => {}; diff --git a/app/assets/javascripts/error_tracking/store/details/getters.js b/app/assets/javascripts/error_tracking/store/details/getters.js new file mode 100644 index 00000000000..7d13439d721 --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/details/getters.js @@ -0,0 +1,3 @@ +export const stacktrace = state => state.stacktraceData.stack_trace_entries.reverse(); + +export default () => {}; diff --git a/app/assets/javascripts/error_tracking/store/details/mutation_types.js b/app/assets/javascripts/error_tracking/store/details/mutation_types.js new file mode 100644 index 00000000000..a2592253a2d --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/details/mutation_types.js @@ -0,0 +1,4 @@ +export const SET_ERROR = 'SET_ERRORS'; +export const SET_LOADING = 'SET_LOADING'; +export const SET_LOADING_STACKTRACE = 'SET_LOADING_STACKTRACE'; +export const SET_STACKTRACE_DATA = 'SET_STACKTRACE_DATA'; diff --git a/app/assets/javascripts/error_tracking/store/details/mutations.js b/app/assets/javascripts/error_tracking/store/details/mutations.js new file mode 100644 index 00000000000..6f4720444e0 --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/details/mutations.js @@ -0,0 +1,16 @@ +import * as types from './mutation_types'; + +export default { + [types.SET_ERROR](state, data) { + state.error = data; + }, + [types.SET_LOADING](state, loading) { + state.loading = loading; + }, + [types.SET_LOADING_STACKTRACE](state, data) { + state.loadingStacktrace = data; + }, + [types.SET_STACKTRACE_DATA](state, data) { + state.stacktraceData = data; + }, +}; diff --git a/app/assets/javascripts/error_tracking/store/details/state.js b/app/assets/javascripts/error_tracking/store/details/state.js new file mode 100644 index 00000000000..95fb0ba0558 --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/details/state.js @@ -0,0 +1,6 @@ +export default () => ({ + error: {}, + stacktraceData: {}, + loading: true, + loadingStacktrace: true, +}); diff --git a/app/assets/javascripts/error_tracking/store/index.js b/app/assets/javascripts/error_tracking/store/index.js index 3ba05e22727..941c752e96a 100644 --- a/app/assets/javascripts/error_tracking/store/index.js +++ b/app/assets/javascripts/error_tracking/store/index.js @@ -1,21 +1,36 @@ import Vue from 'vue'; import Vuex from 'vuex'; -import * as actions from './actions'; -import * as getters from './getters'; -import mutations from './mutations'; + +import * as listActions from './list/actions'; +import listMutations from './list/mutations'; +import listState from './list/state'; +import * as listGetters from './list/getters'; + +import * as detailsActions from './details/actions'; +import detailsMutations from './details/mutations'; +import detailsState from './details/state'; +import * as detailsGetters from './details/getters'; Vue.use(Vuex); export const createStore = () => new Vuex.Store({ - state: { - errors: [], - externalUrl: '', - loading: true, + modules: { + list: { + namespaced: true, + state: listState(), + actions: listActions, + mutations: listMutations, + getters: listGetters, + }, + details: { + namespaced: true, + state: detailsState(), + actions: detailsActions, + mutations: detailsMutations, + getters: detailsGetters, + }, }, - actions, - mutations, - getters, }); export default createStore(); diff --git a/app/assets/javascripts/error_tracking/store/actions.js b/app/assets/javascripts/error_tracking/store/list/actions.js index 1e754a4f54f..18c6e5e9695 100644 --- a/app/assets/javascripts/error_tracking/store/actions.js +++ b/app/assets/javascripts/error_tracking/store/list/actions.js @@ -1,4 +1,4 @@ -import Service from '../services'; +import Service from '../../services'; import * as types from './mutation_types'; import createFlash from '~/flash'; import Poll from '~/lib/utils/poll'; @@ -9,7 +9,7 @@ let eTagPoll; export function startPolling({ commit, dispatch }, endpoint) { eTagPoll = new Poll({ resource: Service, - method: 'getErrorList', + method: 'getSentryData', data: { endpoint }, successCallback: ({ data }) => { if (!data) { diff --git a/app/assets/javascripts/error_tracking/store/getters.js b/app/assets/javascripts/error_tracking/store/list/getters.js index 1a2ec62f79f..1a2ec62f79f 100644 --- a/app/assets/javascripts/error_tracking/store/getters.js +++ b/app/assets/javascripts/error_tracking/store/list/getters.js diff --git a/app/assets/javascripts/error_tracking/store/mutation_types.js b/app/assets/javascripts/error_tracking/store/list/mutation_types.js index f9d77a6b08e..f9d77a6b08e 100644 --- a/app/assets/javascripts/error_tracking/store/mutation_types.js +++ b/app/assets/javascripts/error_tracking/store/list/mutation_types.js diff --git a/app/assets/javascripts/error_tracking/store/mutations.js b/app/assets/javascripts/error_tracking/store/list/mutations.js index e4bd81db9c9..e4bd81db9c9 100644 --- a/app/assets/javascripts/error_tracking/store/mutations.js +++ b/app/assets/javascripts/error_tracking/store/list/mutations.js diff --git a/app/assets/javascripts/error_tracking/store/list/state.js b/app/assets/javascripts/error_tracking/store/list/state.js new file mode 100644 index 00000000000..d371350ef0e --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/list/state.js @@ -0,0 +1,5 @@ +export default () => ({ + errors: [], + externalUrl: '', + loading: true, +}); diff --git a/app/assets/javascripts/pages/projects/error_tracking/details/index.js b/app/assets/javascripts/pages/projects/error_tracking/details/index.js new file mode 100644 index 00000000000..25d1c744e1b --- /dev/null +++ b/app/assets/javascripts/pages/projects/error_tracking/details/index.js @@ -0,0 +1,5 @@ +import ErrorTrackingDetails from '~/error_tracking/details'; + +document.addEventListener('DOMContentLoaded', () => { + ErrorTrackingDetails(); +}); diff --git a/app/assets/javascripts/pages/projects/error_tracking/index.js b/app/assets/javascripts/pages/projects/error_tracking/index.js deleted file mode 100644 index 5a8fe137e9a..00000000000 --- a/app/assets/javascripts/pages/projects/error_tracking/index.js +++ /dev/null @@ -1,5 +0,0 @@ -import ErrorTracking from '~/error_tracking'; - -document.addEventListener('DOMContentLoaded', () => { - ErrorTracking(); -}); diff --git a/app/assets/javascripts/pages/projects/error_tracking/index/index.js b/app/assets/javascripts/pages/projects/error_tracking/index/index.js new file mode 100644 index 00000000000..ead81cd5d2d --- /dev/null +++ b/app/assets/javascripts/pages/projects/error_tracking/index/index.js @@ -0,0 +1,5 @@ +import ErrorTrackingList from '~/error_tracking/list'; + +document.addEventListener('DOMContentLoaded', () => { + ErrorTrackingList(); +}); diff --git a/app/assets/stylesheets/pages/error_details.scss b/app/assets/stylesheets/pages/error_details.scss new file mode 100644 index 00000000000..0515db914e9 --- /dev/null +++ b/app/assets/stylesheets/pages/error_details.scss @@ -0,0 +1,18 @@ +.error-details { + li { + @include gl-line-height-32; + } +} + +.stacktrace { + .file-title { + svg { + vertical-align: middle; + top: -1px; + } + } + + .line_content.old::before { + content: none !important; + } +} diff --git a/app/controllers/admin/abuse_reports_controller.rb b/app/controllers/admin/abuse_reports_controller.rb index d5537023b26..c26c994cf5f 100644 --- a/app/controllers/admin/abuse_reports_controller.rb +++ b/app/controllers/admin/abuse_reports_controller.rb @@ -4,7 +4,7 @@ class Admin::AbuseReportsController < Admin::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def index @abuse_reports = AbuseReport.order(id: :desc).page(params[:page]) - @abuse_reports.includes(:reporter, :user) + @abuse_reports = @abuse_reports.includes(:user, :reporter) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/views/projects/error_tracking/details.html.haml b/app/views/projects/error_tracking/details.html.haml index 52e748fa88b..640746ad8f6 100644 --- a/app/views/projects/error_tracking/details.html.haml +++ b/app/views/projects/error_tracking/details.html.haml @@ -1,3 +1,4 @@ - page_title _('Error Details') +- add_to_breadcrumbs 'Errors', project_error_tracking_index_path(@project) -#js-error_tracking{ data: error_details_data(@current_user, @project) } +#js-error_details{ data: error_details_data(@current_user, @project) } diff --git a/changelogs/unreleased/32464-detail-view-of-sentry-error.yml b/changelogs/unreleased/32464-detail-view-of-sentry-error.yml new file mode 100644 index 00000000000..d23a2b08419 --- /dev/null +++ b/changelogs/unreleased/32464-detail-view-of-sentry-error.yml @@ -0,0 +1,5 @@ +--- +title: Detail view of Sentry error in GitLab +merge_request: 18878 +author: +type: added diff --git a/changelogs/unreleased/34258-embedding-sentry-stacktrace.yml b/changelogs/unreleased/34258-embedding-sentry-stacktrace.yml new file mode 100644 index 00000000000..8cbd16a0e40 --- /dev/null +++ b/changelogs/unreleased/34258-embedding-sentry-stacktrace.yml @@ -0,0 +1,5 @@ +--- +title: Sentry error stacktrace +merge_request: 19492 +author: +type: added diff --git a/changelogs/unreleased/dz-add-indices-to-abuse-reports.yml b/changelogs/unreleased/dz-add-indices-to-abuse-reports.yml new file mode 100644 index 00000000000..8ea5dfce12d --- /dev/null +++ b/changelogs/unreleased/dz-add-indices-to-abuse-reports.yml @@ -0,0 +1,5 @@ +--- +title: Improve performance of admin/abuse_reports page +merge_request: 19630 +author: +type: performance diff --git a/changelogs/unreleased/sh-add-api-exception-to-logs.yml b/changelogs/unreleased/sh-add-api-exception-to-logs.yml new file mode 100644 index 00000000000..273f342d8a1 --- /dev/null +++ b/changelogs/unreleased/sh-add-api-exception-to-logs.yml @@ -0,0 +1,5 @@ +--- +title: Include exception and backtrace in API logs +merge_request: 19671 +author: +type: other diff --git a/db/post_migrate/20191105140942_add_indices_to_abuse_reports.rb b/db/post_migrate/20191105140942_add_indices_to_abuse_reports.rb new file mode 100644 index 00000000000..2b2d04e8ccc --- /dev/null +++ b/db/post_migrate/20191105140942_add_indices_to_abuse_reports.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndicesToAbuseReports < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :abuse_reports, :user_id + end + + def down + remove_concurrent_index :abuse_reports, :user_id + end +end diff --git a/db/schema.rb b/db/schema.rb index c0b74841834..a43e1c70ad5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_11_05_094625) do +ActiveRecord::Schema.define(version: 2019_11_05_140942) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -24,6 +24,7 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do t.datetime "updated_at" t.text "message_html" t.integer "cached_markdown_version" + t.index ["user_id"], name: "index_abuse_reports_on_user_id" end create_table "alerts_service_data", force: :cascade do |t| diff --git a/doc/administration/geo/replication/version_specific_updates.md b/doc/administration/geo/replication/version_specific_updates.md index 6d550a49df4..5288cc6e186 100644 --- a/doc/administration/geo/replication/version_specific_updates.md +++ b/doc/administration/geo/replication/version_specific_updates.md @@ -4,6 +4,24 @@ Check this document if it includes instructions for the version you are updating These steps go together with the [general steps](updating_the_geo_nodes.md#general-update-steps) for updating Geo nodes. +## Updating to GitLab 12.2 + +GitLab 12.2 includes the following minor PostgreSQL updates: + +- To version `9.6.14` if you run PostgreSQL 9.6. +- To version `10.9` if you run PostgreSQL 10. + +This update will occur even if major PostgreSQL updates are disabled. + +Before [refreshing Foreign Data Wrapper during a Geo HA upgrade](https://docs.gitlab.com/omnibus/update/README.html#run-post-deployment-migrations-and-checks), +restart the Geo tracking database: + +```sh +sudo gitlab-ctl restart geo-postgresql +``` + +The restart avoids a version mismatch when PostgreSQL tries to load the FDW extension. + ## Updating to GitLab 12.1 By default, GitLab 12.1 will attempt to automatically update the diff --git a/lib/api/api.rb b/lib/api/api.rb index d71f0c38ce6..0062759d993 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -21,6 +21,7 @@ module API Gitlab::GrapeLogging::Loggers::ClientEnvLogger.new, Gitlab::GrapeLogging::Loggers::RouteLogger.new, Gitlab::GrapeLogging::Loggers::UserLogger.new, + Gitlab::GrapeLogging::Loggers::ExceptionLogger.new, Gitlab::GrapeLogging::Loggers::QueueDurationLogger.new, Gitlab::GrapeLogging::Loggers::PerfLogger.new, Gitlab::GrapeLogging::Loggers::CorrelationIdLogger.new diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 19c29847ce3..e740134f85f 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -9,6 +9,7 @@ module API GITLAB_SHARED_SECRET_HEADER = "Gitlab-Shared-Secret" SUDO_PARAM = :sudo API_USER_ENV = 'gitlab.api.user' + API_EXCEPTION_ENV = 'gitlab.api.exception' def declared_params(options = {}) options = { include_parent_namespaces: false }.merge(options) @@ -387,6 +388,9 @@ module API Gitlab::Sentry.track_acceptable_exception(exception, extra: params) end + # This is used with GrapeLogging::Loggers::ExceptionLogger + env[API_EXCEPTION_ENV] = exception + # lifted from https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb#L60 trace = exception.backtrace diff --git a/lib/gitlab/grape_logging/loggers/exception_logger.rb b/lib/gitlab/grape_logging/loggers/exception_logger.rb new file mode 100644 index 00000000000..022eb15d28d --- /dev/null +++ b/lib/gitlab/grape_logging/loggers/exception_logger.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Gitlab + module GrapeLogging + module Loggers + class ExceptionLogger < ::GrapeLogging::Loggers::Base + def parameters(request, _) + # grape-logging attempts to pass the logger the exception + # (https://github.com/aserafin/grape_logging/blob/v1.7.0/lib/grape_logging/middleware/request_logger.rb#L63), + # but it appears that the rescue_all in api.rb takes + # precedence so the logger never sees it. We need to + # store and retrieve the exception from the environment. + exception = request.env[::API::Helpers::API_EXCEPTION_ENV] + + return {} unless exception.is_a?(Exception) + + data = { + exception: { + class: exception.class.to_s, + message: exception.message + } + } + + if exception.backtrace + data[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(exception.backtrace) + end + + data + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bb22b7cbe18..4601b7dffd0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6625,6 +6625,9 @@ msgstr "" msgid "Error deleting %{issuableType}" msgstr "" +msgid "Error details" +msgstr "" + msgid "Error fetching diverging counts for branches. Please try again." msgstr "" @@ -7057,6 +7060,9 @@ msgstr "" msgid "Failed to load emoji list." msgstr "" +msgid "Failed to load error details from Sentry." +msgstr "" + msgid "Failed to load errors from Sentry. Error message: %{errorMessage}" msgstr "" @@ -7066,6 +7072,9 @@ msgstr "" msgid "Failed to load related branches" msgstr "" +msgid "Failed to load stacktrace." +msgstr "" + msgid "Failed to mark this issue as a duplicate because referenced issue was not found." msgstr "" @@ -7455,6 +7464,9 @@ msgstr "" msgid "First name" msgstr "" +msgid "First seen" +msgstr "" + msgid "Fixed date" msgstr "" @@ -14222,6 +14234,9 @@ msgstr "" msgid "Report abuse to admin" msgstr "" +msgid "Reported %{timeAgo} by %{reportedBy}" +msgstr "" + msgid "Reporting" msgstr "" @@ -15198,6 +15213,9 @@ msgstr "" msgid "Sentry API URL" msgstr "" +msgid "Sentry event" +msgstr "" + msgid "Sep" msgstr "" @@ -16022,6 +16040,9 @@ msgstr "" msgid "Squash commits" msgstr "" +msgid "Stack trace" +msgstr "" + msgid "Stage" msgstr "" diff --git a/qa/qa/vendor/github/page/login.rb b/qa/qa/vendor/github/page/login.rb index f6e72bb01f9..232c8743de7 100644 --- a/qa/qa/vendor/github/page/login.rb +++ b/qa/qa/vendor/github/page/login.rb @@ -12,11 +12,15 @@ module QA fill_in 'password', with: QA::Runtime::Env.github_password click_on 'Sign in' - otp = OnePassword::CLI.new.otp + Support::Retrier.retry_until(exit_on_failure: true) do + otp = OnePassword::CLI.new.otp - fill_in 'otp', with: otp + fill_in 'otp', with: otp - click_on 'Verify' + click_on 'Verify' + + !has_text?('Two-factor authentication failed', wait: 1.0) + end click_on 'Authorize gitlab-qa' if has_button?('Authorize gitlab-qa') end diff --git a/spec/frontend/error_tracking/components/error_details_spec.js b/spec/frontend/error_tracking/components/error_details_spec.js new file mode 100644 index 00000000000..54e8b0848a2 --- /dev/null +++ b/spec/frontend/error_tracking/components/error_details_spec.js @@ -0,0 +1,105 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import Vuex from 'vuex'; +import { GlLoadingIcon, GlLink } from '@gitlab/ui'; +import Stacktrace from '~/error_tracking/components/stacktrace.vue'; +import ErrorDetails from '~/error_tracking/components/error_details.vue'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('ErrorDetails', () => { + let store; + let wrapper; + let actions; + let getters; + + function mountComponent() { + wrapper = shallowMount(ErrorDetails, { + localVue, + store, + propsData: { + issueDetailsPath: '/123/details', + issueStackTracePath: '/stacktrace', + }, + }); + } + + beforeEach(() => { + actions = { + startPollingDetails: () => {}, + startPollingStacktrace: () => {}, + }; + + getters = { + sentryUrl: () => 'sentry.io', + stacktrace: () => [{ context: [1, 2], lineNo: 53, filename: 'index.js' }], + }; + + const state = { + error: {}, + loading: true, + stacktraceData: {}, + loadingStacktrace: true, + }; + + store = new Vuex.Store({ + modules: { + details: { + namespaced: true, + actions, + state, + getters, + }, + }, + }); + }); + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + describe('loading', () => { + beforeEach(() => { + mountComponent(); + }); + + it('should show spinner while loading', () => { + expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); + expect(wrapper.find(GlLink).exists()).toBe(false); + expect(wrapper.find(Stacktrace).exists()).toBe(false); + }); + }); + + describe('Error details', () => { + it('should show Sentry error details without stacktrace', () => { + store.state.details.loading = false; + store.state.details.error.id = 1; + mountComponent(); + expect(wrapper.find(GlLink).exists()).toBe(true); + expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); + expect(wrapper.find(Stacktrace).exists()).toBe(false); + }); + + describe('Stacktrace', () => { + it('should show stacktrace', () => { + store.state.details.loading = false; + store.state.details.error.id = 1; + store.state.details.loadingStacktrace = false; + mountComponent(); + expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); + expect(wrapper.find(Stacktrace).exists()).toBe(true); + }); + + it('should NOT show stacktrace if no entries', () => { + store.state.details.loading = false; + store.state.details.loadingStacktrace = false; + store.getters = { 'details/sentryUrl': () => 'sentry.io', 'details/stacktrace': () => [] }; + mountComponent(); + expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); + expect(wrapper.find(Stacktrace).exists()).toBe(false); + }); + }); + }); +}); diff --git a/spec/frontend/error_tracking/components/error_tracking_list_spec.js b/spec/frontend/error_tracking/components/error_tracking_list_spec.js index ce8b8908026..1bbf23cc602 100644 --- a/spec/frontend/error_tracking/components/error_tracking_list_spec.js +++ b/spec/frontend/error_tracking/components/error_tracking_list_spec.js @@ -34,7 +34,7 @@ describe('ErrorTrackingList', () => { beforeEach(() => { actions = { - getErrorList: () => {}, + getSentryData: () => {}, startPolling: () => {}, restartPolling: jest.fn().mockName('restartPolling'), }; @@ -45,8 +45,13 @@ describe('ErrorTrackingList', () => { }; store = new Vuex.Store({ - actions, - state, + modules: { + list: { + namespaced: true, + actions, + state, + }, + }, }); }); @@ -70,7 +75,7 @@ describe('ErrorTrackingList', () => { describe('results', () => { beforeEach(() => { - store.state.loading = false; + store.state.list.loading = false; mountComponent(); }); @@ -84,7 +89,7 @@ describe('ErrorTrackingList', () => { describe('no results', () => { beforeEach(() => { - store.state.loading = false; + store.state.list.loading = false; mountComponent(); }); diff --git a/spec/frontend/error_tracking/components/stacktrace_entry_spec.js b/spec/frontend/error_tracking/components/stacktrace_entry_spec.js new file mode 100644 index 00000000000..95958408770 --- /dev/null +++ b/spec/frontend/error_tracking/components/stacktrace_entry_spec.js @@ -0,0 +1,49 @@ +import { shallowMount } from '@vue/test-utils'; +import StackTraceEntry from '~/error_tracking/components/stacktrace_entry.vue'; +import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; +import Icon from '~/vue_shared/components/icon.vue'; + +describe('Stacktrace Entry', () => { + let wrapper; + + function mountComponent(props) { + wrapper = shallowMount(StackTraceEntry, { + propsData: { + filePath: 'sidekiq/util.rb', + lines: [ + [22, ' def safe_thread(name, \u0026block)\n'], + [23, ' Thread.new do\n'], + [24, " Thread.current['sidekiq_label'] = name\n"], + [25, ' watchdog(name, \u0026block)\n'], + ], + errorLine: 24, + ...props, + }, + }); + } + + beforeEach(() => { + mountComponent(); + }); + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + it('should render stacktrace entry collapsed', () => { + expect(wrapper.find(StackTraceEntry).exists()).toBe(true); + expect(wrapper.find(ClipboardButton).exists()).toBe(true); + expect(wrapper.find(Icon).exists()).toBe(true); + expect(wrapper.find(FileIcon).exists()).toBe(true); + expect(wrapper.element.querySelectorAll('table').length).toBe(0); + }); + + it('should render stacktrace entry table expanded', () => { + mountComponent({ expanded: true }); + expect(wrapper.element.querySelectorAll('tr.line_holder').length).toBe(4); + expect(wrapper.element.querySelectorAll('.line_content.old').length).toBe(1); + }); +}); diff --git a/spec/frontend/error_tracking/components/stacktrace_spec.js b/spec/frontend/error_tracking/components/stacktrace_spec.js new file mode 100644 index 00000000000..4f4a60acba4 --- /dev/null +++ b/spec/frontend/error_tracking/components/stacktrace_spec.js @@ -0,0 +1,45 @@ +import { shallowMount } from '@vue/test-utils'; +import Stacktrace from '~/error_tracking/components/stacktrace.vue'; +import StackTraceEntry from '~/error_tracking/components/stacktrace_entry.vue'; + +describe('ErrorDetails', () => { + let wrapper; + + const stackTraceEntry = { + filename: 'sidekiq/util.rb', + context: [ + [22, ' def safe_thread(name, \u0026block)\n'], + [23, ' Thread.new do\n'], + [24, " Thread.current['sidekiq_label'] = name\n"], + [25, ' watchdog(name, \u0026block)\n'], + ], + lineNo: 24, + }; + + function mountComponent(entries) { + wrapper = shallowMount(Stacktrace, { + propsData: { + entries, + }, + }); + } + + describe('Stacktrace', () => { + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + it('should render single Stacktrace entry', () => { + mountComponent([stackTraceEntry]); + expect(wrapper.findAll(StackTraceEntry).length).toBe(1); + }); + + it('should render multiple Stacktrace entry', () => { + const entriesNum = 3; + mountComponent(new Array(entriesNum).fill(stackTraceEntry)); + expect(wrapper.findAll(StackTraceEntry).length).toBe(entriesNum); + }); + }); +}); diff --git a/spec/frontend/error_tracking/store/details/actions_spec.js b/spec/frontend/error_tracking/store/details/actions_spec.js new file mode 100644 index 00000000000..f72cd1e413b --- /dev/null +++ b/spec/frontend/error_tracking/store/details/actions_spec.js @@ -0,0 +1,94 @@ +import axios from '~/lib/utils/axios_utils'; +import MockAdapter from 'axios-mock-adapter'; +import testAction from 'helpers/vuex_action_helper'; +import createFlash from '~/flash'; +import * as actions from '~/error_tracking/store/details/actions'; +import * as types from '~/error_tracking/store/details/mutation_types'; + +jest.mock('~/flash.js'); +let mock; + +describe('Sentry error details store actions', () => { + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + createFlash.mockClear(); + }); + + describe('startPollingDetails', () => { + const endpoint = '123/details'; + it('should commit SET_ERROR with received response', done => { + const payload = { error: { id: 1 } }; + mock.onGet().reply(200, payload); + testAction( + actions.startPollingDetails, + { endpoint }, + {}, + [ + { type: types.SET_ERROR, payload: payload.error }, + { type: types.SET_LOADING, payload: false }, + ], + [], + () => { + done(); + }, + ); + }); + + it('should show flash on API error', done => { + mock.onGet().reply(400); + + testAction( + actions.startPollingDetails, + { endpoint }, + {}, + [{ type: types.SET_LOADING, payload: false }], + [], + () => { + expect(createFlash).toHaveBeenCalledTimes(1); + done(); + }, + ); + }); + }); + + describe('startPollingStacktrace', () => { + const endpoint = '123/stacktrace'; + it('should commit SET_ERROR with received response', done => { + const payload = { error: [1, 2, 3] }; + mock.onGet().reply(200, payload); + testAction( + actions.startPollingStacktrace, + { endpoint }, + {}, + [ + { type: types.SET_STACKTRACE_DATA, payload: payload.error }, + { type: types.SET_LOADING_STACKTRACE, payload: false }, + ], + [], + () => { + done(); + }, + ); + }); + + it('should show flash on API error', done => { + mock.onGet().reply(400); + + testAction( + actions.startPollingStacktrace, + { endpoint }, + {}, + [{ type: types.SET_LOADING_STACKTRACE, payload: false }], + [], + () => { + expect(createFlash).toHaveBeenCalledTimes(1); + done(); + }, + ); + }); + }); +}); diff --git a/spec/frontend/error_tracking/store/details/getters_spec.js b/spec/frontend/error_tracking/store/details/getters_spec.js new file mode 100644 index 00000000000..ea57de5872b --- /dev/null +++ b/spec/frontend/error_tracking/store/details/getters_spec.js @@ -0,0 +1,13 @@ +import * as getters from '~/error_tracking/store/details/getters'; + +describe('Sentry error details store getters', () => { + const state = { + stacktraceData: { stack_trace_entries: [1, 2] }, + }; + + describe('stacktrace', () => { + it('should get stacktrace', () => { + expect(getters.stacktrace(state)).toEqual([2, 1]); + }); + }); +}); diff --git a/spec/frontend/error_tracking/store/getters_spec.js b/spec/frontend/error_tracking/store/list/getters_spec.js index 371dfae373b..3cd7fa37d44 100644 --- a/spec/frontend/error_tracking/store/getters_spec.js +++ b/spec/frontend/error_tracking/store/list/getters_spec.js @@ -1,4 +1,4 @@ -import * as getters from '~/error_tracking/store/getters'; +import * as getters from '~/error_tracking/store/list/getters'; describe('Error Tracking getters', () => { let state; diff --git a/spec/frontend/error_tracking/store/mutation_spec.js b/spec/frontend/error_tracking/store/list/mutation_spec.js index 8117104bdbc..6e021185b4d 100644 --- a/spec/frontend/error_tracking/store/mutation_spec.js +++ b/spec/frontend/error_tracking/store/list/mutation_spec.js @@ -1,5 +1,5 @@ -import mutations from '~/error_tracking/store/mutations'; -import * as types from '~/error_tracking/store/mutation_types'; +import mutations from '~/error_tracking/store/list/mutations'; +import * as types from '~/error_tracking/store/list/mutation_types'; describe('Error tracking mutations', () => { describe('SET_ERRORS', () => { diff --git a/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb b/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb new file mode 100644 index 00000000000..8d7826c0a56 --- /dev/null +++ b/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Gitlab::GrapeLogging::Loggers::ExceptionLogger do + subject { described_class.new } + + let(:mock_request) { OpenStruct.new(env: {}) } + + describe ".parameters" do + describe 'when no exception is available' do + it 'returns an empty hash' do + expect(subject.parameters(mock_request, nil)).to eq({}) + end + end + + describe 'when an exception is available' do + let(:exception) { RuntimeError.new('This is a test') } + let(:mock_request) do + OpenStruct.new( + env: { + ::API::Helpers::API_EXCEPTION_ENV => exception + } + ) + end + + let(:expected) do + { + exception: { + class: 'RuntimeError', + message: 'This is a test' + } + } + end + + it 'returns the correct fields' do + expect(subject.parameters(mock_request, nil)).to eq(expected) + end + + context 'with backtrace' do + before do + current_backtrace = caller + allow(exception).to receive(:backtrace).and_return(current_backtrace) + expected[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(current_backtrace) + end + + it 'includes the backtrace' do + expect(subject.parameters(mock_request, nil)).to eq(expected) + end + end + end + end +end |