-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Hotfix/UI bug #576
Hotfix/UI bug #576
Conversation
ですので、今後デバッグするときは |
この問題 は再現しませんでしたか? |
Type Check でこれが防げ寝なかったのもきになりますね 追記: |
再現できなかったですね。 |
過去の そして今使っている vue のバージョンは低いので、これに該当する可能性があります。 またそもそも当時は |
|
||
if (module === "Unknown" || day === "Unknown") return; | ||
if (isSpecialDay(day)) { | ||
specialTimetable[module][day] = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
僕のタグページが落ちる原因がわかりました。
僕の場合ここの module
に "Annual"
が入ってくるので以下のようにタイプエラーになります。
TypeError: Cannot set properties of undefined (setting 'AnyTime')
at schedule.ts:55:32
at Array.forEach (<anonymous>)
at uE (schedule.ts:49:16)
at dE (registeredCourse.ts:18:32)
at Array.map (<anonymous>)
at CourseRepository.ts:207:50
at GS.call (index.ts:97:16)
at async mR (course.ts:46:9)
at async Promise.all (index 1)
at async hR (course.ts:40:30)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ただ open spec を見る限り、module
の "Annnual"
は廃止され、新たにisAnnual
みたいなプロパティが付与されていた気がします。
なのでこの問題はフロントよりもバックエンドの問題かもしれませんね…
ただ僕と同じような人はみんなこのページが落ちそうなので、フロントで対処療法的に対応するか、バックエンドに修正して貰う必要がありそうです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const { | ||
body, | ||
originalResponse, | ||
status, | ||
}: ApiRespoinse<AR, SS | FS> = await api(this.#client); | ||
|
||
if (isContained(status, successStatusList)) { | ||
return callback(body); | ||
} | ||
|
||
if (isContained(status, failedStatusList)) { | ||
return this.#handleApiFailedStatus(status, originalResponse); | ||
} | ||
|
||
return new InternalServerError(`Invalid Status : ${status}`); | ||
} catch { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この PR は HotFix なので、これは今対応しなくてもいい問題だと思います。ただ忘れないように書き残します。
ここでの throw を NetworkError
とまとめてしまうのはユーザにとっても開発者にとっても不都合かなと思いました。
なぜならここの部分は NetworkError
ではない可能性があるためです。
今回の場合 await api(this.#client);
では Type Error が発生していました。しかし例外時に NetworkError()
としていることで、ネットワークエラーのえらーメッセージが表示されています
その結果、ユーザにとってはネットワークエラーではないのにネットワークエラーと表示され混乱を与えます。
開発者にとってもネットワークエラーという誤ったエラー報告を受けることでバグの特定が遅れます。
個人的にはもっと通信に関わる部分のみで try {} catch {}
したさがあります。
具体的には await api(this.#client);
の部分のみですかね…
後述するように自分の環境の問題の可能性が高いので無視してよさそう |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
授業を検索した時に表示される授業の科目番号がuuidになっている
授業詳細画面に遷移するとbtnRef is not definedとエラーが発生する
以上の問題修正されていること確認しました、ありがとうございます!
#576 (comment) |
そうだと思います。 |
4cdf570
to
e30e4b0
Compare
Annualの件修正したので、確認してもらえると助かります。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます!!
単位数ページでバグが発生しないことを確認できました!
以下の手順でcommitをしました。
修正したバグは以下の2点です。
btnRef is not defined
とエラーが発生する