Skip to content

extension fixes to extract document history#204

Merged
bradley-erickson merged 3 commits intomasterfrom
berickson/202412-extension-fixes
Dec 13, 2024
Merged

extension fixes to extract document history#204
bradley-erickson merged 3 commits intomasterfrom
berickson/202412-extension-fixes

Conversation

@bradley-erickson
Copy link
Collaborator

No description provided.

@pmitros
Copy link
Contributor

pmitros commented Dec 12, 2024

This looks good to me, except for the lack of error handling. If there is no docs history, the exception should be sent to the server in place of the docs history. This most likely means we either return a token with the docs history or an error token.

Another is to raise an exception and to propagate it up to where we request the Google Docs history.

But in either case, we should send something to lo_event, either with the history, or with as much information as we have about why it's not there for future debugging.

This is important since this code is brittle. We are parsing Google JS which might change. It's also helpful to document why this is brittle and what we're doing.

In either case, with those changes, it looks good for a merge. Without those changes -- if there is time pressure -- it still makes sense to merge since the existing code is broken.

@bradley-erickson bradley-erickson merged commit 5020beb into master Dec 13, 2024
1 check failed
bradley-erickson added a commit to ArgLab/ArgLab_writing_observer that referenced this pull request Oct 7, 2025
* fixed bug with inject.js that caused issues with the google CSP

* added the files this time

* pr feedback
DrLynch pushed a commit to ArgLab/ArgLab_writing_observer that referenced this pull request Oct 10, 2025
* fixed bug with inject.js that caused issues with the google CSP

* added the files this time

* pr feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants