Fix error handling and write-ordering anti-patterns

Return errors to clients on write failures instead of silently succeeding.
Add rollback logic to BanFile and ThreadedNewsYAML mutations so in-memory
state is restored when persistence fails. Extract error message constants
and add comprehensive tests for error paths and rollback behavior.

6 files changed, 775 insertions, 70 deletions

diff --git a/internal/mobius/ban.go b/internal/mobius/ban.go
index 478dbb0..6ccb9fe 100644
--- a/internal/mobius/ban.go
+++ b/internal/mobius/ban.go
@@ -97,8 +97,19 @@ func (bf *BanFile) Load() error {
 
 // add is the internal implementation that assumes the caller holds the lock.
 func (bf *BanFile) add(ip string, until *time.Time) error {
+	old, existed := bf.banList[ip]
 	bf.banList[ip] = until
-	return bf.save()
+
+	if err := bf.save(); err != nil {
+		if existed {
+			bf.banList[ip] = old
+		} else {
+			delete(bf.banList, ip)
+		}
+		return err
+	}
+
+	return nil
 }
 
 func (bf *BanFile) Add(ip string, until *time.Time) error {
@@ -146,8 +157,17 @@ func (bf *BanFile) UnbanIP(ip string) error {
 	bf.Lock()
 	defer bf.Unlock()
 
+	old, existed := bf.banList[ip]
 	delete(bf.banList, ip)
-	return bf.save()
+
+	if err := bf.save(); err != nil {
+		if existed {
+			bf.banList[ip] = old
+		}
+		return err
+	}
+
+	return nil
 }
 
 // BanUsername adds a username to the banned users set
@@ -155,8 +175,19 @@ func (bf *BanFile) BanUsername(username string) error {
 	bf.Lock()
 	defer bf.Unlock()
 
+	old, existed := bf.bannedUsers[username]
 	bf.bannedUsers[username] = true
-	return bf.save()
+
+	if err := bf.save(); err != nil {
+		if existed {
+			bf.bannedUsers[username] = old
+		} else {
+			delete(bf.bannedUsers, username)
+		}
+		return err
+	}
+
+	return nil
 }
 
 // UnbanUsername removes a username from the banned users set
@@ -164,8 +195,17 @@ func (bf *BanFile) UnbanUsername(username string) error {
 	bf.Lock()
 	defer bf.Unlock()
 
+	old, existed := bf.bannedUsers[username]
 	delete(bf.bannedUsers, username)
-	return bf.save()
+
+	if err := bf.save(); err != nil {
+		if existed {
+			bf.bannedUsers[username] = old
+		}
+		return err
+	}
+
+	return nil
 }
 
 // IsUsernameBanned checks if a username is banned
@@ -181,8 +221,19 @@ func (bf *BanFile) BanNickname(nickname string) error {
 	bf.Lock()
 	defer bf.Unlock()
 
+	old, existed := bf.bannedNicks[nickname]
 	bf.bannedNicks[nickname] = true
-	return bf.save()
+
+	if err := bf.save(); err != nil {
+		if existed {
+			bf.bannedNicks[nickname] = old
+		} else {
+			delete(bf.bannedNicks, nickname)
+		}
+		return err
+	}
+
+	return nil
 }
 
 // UnbanNickname removes a nickname from the banned nicknames set
@@ -190,8 +241,17 @@ func (bf *BanFile) UnbanNickname(nickname string) error {
 	bf.Lock()
 	defer bf.Unlock()
 
+	old, existed := bf.bannedNicks[nickname]
 	delete(bf.bannedNicks, nickname)
-	return bf.save()
+
+	if err := bf.save(); err != nil {
+		if existed {
+			bf.bannedNicks[nickname] = old
+		}
+		return err
+	}
+
+	return nil
 }
 
 // IsNicknameBanned checks if a nickname is banned
diff --git a/internal/mobius/ban_test.go b/internal/mobius/ban_test.go
index 877fb62..c7535ac 100644
--- a/internal/mobius/ban_test.go
+++ b/internal/mobius/ban_test.go
@@ -273,6 +273,50 @@ func TestBanFile_PermanentBanViaAdd(t *testing.T) {
 	assert.Nil(t, until)
 }
 
+func TestBanFile_Add_rollback(t *testing.T) {
+	bf := &BanFile{
+		filePath:    "/nonexistent/dir/banfile.yaml",
+		banList:     make(map[string]*time.Time),
+		bannedUsers: make(map[string]bool),
+		bannedNicks: make(map[string]bool),
+	}
+
+	err := bf.Add("1.2.3.4", nil)
+	assert.Error(t, err)
+
+	_, exists := bf.banList["1.2.3.4"]
+	assert.False(t, exists, "IP should not be in banList after save failure")
+}
+
+func TestBanFile_UnbanIP_rollback(t *testing.T) {
+	bf := &BanFile{
+		filePath:    "/nonexistent/dir/banfile.yaml",
+		banList:     map[string]*time.Time{"10.0.0.1": nil},
+		bannedUsers: make(map[string]bool),
+		bannedNicks: make(map[string]bool),
+	}
+
+	err := bf.UnbanIP("10.0.0.1")
+	assert.Error(t, err)
+
+	_, exists := bf.banList["10.0.0.1"]
+	assert.True(t, exists, "IP should still be in banList after save failure")
+}
+
+func TestBanFile_BanNickname_rollback(t *testing.T) {
+	bf := &BanFile{
+		filePath:    "/nonexistent/dir/banfile.yaml",
+		banList:     make(map[string]*time.Time),
+		bannedUsers: make(map[string]bool),
+		bannedNicks: make(map[string]bool),
+	}
+
+	err := bf.BanNickname("troll")
+	assert.Error(t, err)
+
+	assert.False(t, bf.bannedNicks["troll"], "nickname should not be in bannedNicks after save failure")
+}
+
 func TestBanFile_NewFormatPersistence(t *testing.T) {
 	bf := newTempBanFile(t)
 
diff --git a/internal/mobius/threaded_news.go b/internal/mobius/threaded_news.go
index eae2349..41ae2cb 100644
--- a/internal/mobius/threaded_news.go
+++ b/internal/mobius/threaded_news.go
@@ -41,7 +41,12 @@ func (n *ThreadedNewsYAML) CreateGrouping(newsPath []string, name string, t [2]b
 		SubCats:  make(map[string]hotline.NewsCategoryListData15),
 	}
 
-	return n.writeFile()
+	if err := n.writeFile(); err != nil {
+		delete(cats, name)
+		return err
+	}
+
+	return nil
 }
 
 func (n *ThreadedNewsYAML) NewsItem(newsPath []string) hotline.NewsCategoryListData15 {
@@ -71,9 +76,15 @@ func (n *ThreadedNewsYAML) DeleteNewsItem(newsPath []string) error {
 		}
 	}
 
+	oldVal := cats[delName]
 	delete(cats, delName)
 
-	return n.writeFile()
+	if err := n.writeFile(); err != nil {
+		cats[delName] = oldVal
+		return err
+	}
+
+	return nil
 }
 
 func (n *ThreadedNewsYAML) GetArticle(newsPath []string, articleID uint32) *hotline.NewsArtData {
@@ -153,13 +164,22 @@ func (n *ThreadedNewsYAML) PostArticle(newsPath []string, parentArticleID uint32
 	}
 
 	nextID := uint32(1)
+	var prevID uint32
+	var oldNextArt [4]byte
+	hasPrev := false
+	setFirstChild := false
+
 	if len(keys) > 0 {
 		sort.Ints(keys)
-		prevID := uint32(keys[len(keys)-1])
+		prevID = uint32(keys[len(keys)-1])
 		nextID = prevID + 1
+		hasPrev = true
 
 		binary.BigEndian.PutUint32(article.PrevArt[:], prevID)
 
+		// Save old value for rollback
+		oldNextArt = cat.Articles[prevID].NextArt
+
 		// Set next article Type
 		binary.BigEndian.PutUint32(cat.Articles[prevID].NextArt[:], nextID)
 	}
@@ -170,6 +190,7 @@ func (n *ThreadedNewsYAML) PostArticle(newsPath []string, parentArticleID uint32
 		parentArt := cat.Articles[parentID]
 
 		if parentArt.FirstChildArt == [4]byte{0, 0, 0, 0} {
+			setFirstChild = true
 			binary.BigEndian.PutUint32(parentArt.FirstChildArt[:], nextID)
 		}
 	}
@@ -178,7 +199,19 @@ func (n *ThreadedNewsYAML) PostArticle(newsPath []string, parentArticleID uint32
 
 	cats[catName] = cat
 
-	return n.writeFile()
+	if err := n.writeFile(); err != nil {
+		delete(cat.Articles, nextID)
+		if hasPrev {
+			cat.Articles[prevID].NextArt = oldNextArt
+		}
+		if setFirstChild {
+			cat.Articles[parentID].FirstChildArt = [4]byte{}
+		}
+		cats[catName] = cat
+		return err
+	}
+
+	return nil
 }
 
 func (n *ThreadedNewsYAML) DeleteArticle(newsPath []string, articleID uint32, _ bool) error {
@@ -198,10 +231,17 @@ func (n *ThreadedNewsYAML) DeleteArticle(newsPath []string, articleID uint32, _
 	catName := newsPath[len(newsPath)-1]
 
 	cat := cats[catName]
+	oldArticle := cat.Articles[articleID]
 	delete(cat.Articles, articleID)
 	cats[catName] = cat
 
-	return n.writeFile()
+	if err := n.writeFile(); err != nil {
+		cat.Articles[articleID] = oldArticle
+		cats[catName] = cat
+		return err
+	}
+
+	return nil
 }
 
 func (n *ThreadedNewsYAML) ListArticles(newsPath []string) (hotline.NewsArtListData, error) {
diff --git a/internal/mobius/threaded_news_test.go b/internal/mobius/threaded_news_test.go
index a44e834..608f5fb 100644
--- a/internal/mobius/threaded_news_test.go
+++ b/internal/mobius/threaded_news_test.go
@@ -213,3 +213,179 @@ func TestThreadedNewsYAML_CreateGrouping(t *testing.T) {
 		})
 	}
 }
+
+func TestThreadedNewsYAML_CreateGrouping_rollback(t *testing.T) {
+	n := &ThreadedNewsYAML{
+		ThreadedNews: hotline.ThreadedNews{
+			Categories: map[string]hotline.NewsCategoryListData15{
+				"Existing": {
+					Name:     "Existing",
+					Type:     hotline.NewsCategory,
+					Articles: make(map[uint32]*hotline.NewsArtData),
+					SubCats:  make(map[string]hotline.NewsCategoryListData15),
+				},
+			},
+		},
+		filePath: "/nonexistent/dir/ThreadedNews.yaml",
+	}
+
+	err := n.CreateGrouping(nil, "NewBundle", hotline.NewsBundle)
+	assert.Error(t, err)
+
+	// The new entry should have been rolled back.
+	_, exists := n.ThreadedNews.Categories["NewBundle"]
+	assert.False(t, exists, "new grouping should be removed on write failure")
+
+	// Existing entry should still be present.
+	_, exists = n.ThreadedNews.Categories["Existing"]
+	assert.True(t, exists, "existing grouping should be preserved")
+}
+
+func TestThreadedNewsYAML_DeleteNewsItem_rollback(t *testing.T) {
+	n := &ThreadedNewsYAML{
+		ThreadedNews: hotline.ThreadedNews{
+			Categories: map[string]hotline.NewsCategoryListData15{
+				"ToDelete": {
+					Name:     "ToDelete",
+					Type:     hotline.NewsCategory,
+					Articles: make(map[uint32]*hotline.NewsArtData),
+					SubCats:  make(map[string]hotline.NewsCategoryListData15),
+				},
+			},
+		},
+		filePath: "/nonexistent/dir/ThreadedNews.yaml",
+	}
+
+	err := n.DeleteNewsItem([]string{"ToDelete"})
+	assert.Error(t, err)
+
+	// The deleted entry should have been restored.
+	cat, exists := n.ThreadedNews.Categories["ToDelete"]
+	assert.True(t, exists, "deleted item should be restored on write failure")
+	assert.Equal(t, "ToDelete", cat.Name)
+}
+
+func TestThreadedNewsYAML_PostArticle_rollback(t *testing.T) {
+	// Set up a category with existing articles (mimics the test fixture).
+	n := &ThreadedNewsYAML{
+		ThreadedNews: hotline.ThreadedNews{
+			Categories: map[string]hotline.NewsCategoryListData15{
+				"TestCat": {
+					Type: hotline.NewsCategory,
+					Name: "TestCat",
+					Articles: map[uint32]*hotline.NewsArtData{
+						1: {
+							Title:         "TestArt",
+							NextArt:       [4]byte{0, 0, 0, 2},
+							FirstChildArt: [4]byte{0, 0, 0, 2},
+						},
+						2: {
+							Title:     "Re: TestArt",
+							PrevArt:   [4]byte{0, 0, 0, 1},
+							ParentArt: [4]byte{0, 0, 0, 1},
+							NextArt:   [4]byte{0, 0, 0, 3},
+						},
+						3: {
+							Title:   "TestArt 2",
+							PrevArt: [4]byte{0, 0, 0, 2},
+						},
+					},
+					SubCats: make(map[string]hotline.NewsCategoryListData15),
+				},
+			},
+		},
+		filePath: "/nonexistent/dir/ThreadedNews.yaml",
+	}
+
+	// Snapshot state before.
+	origNextArt3 := n.ThreadedNews.Categories["TestCat"].Articles[3].NextArt
+
+	newArticle := hotline.NewsArtData{
+		Title:  "New Article",
+		Poster: "tester",
+	}
+
+	err := n.PostArticle([]string{"TestCat"}, 0, newArticle)
+	assert.Error(t, err)
+
+	cat := n.ThreadedNews.Categories["TestCat"]
+
+	// New article (ID 4) should not exist.
+	_, exists := cat.Articles[4]
+	assert.False(t, exists, "new article should be removed on write failure")
+
+	// Article 3's NextArt should be restored to its original value.
+	assert.Equal(t, origNextArt3, cat.Articles[3].NextArt, "previous article's NextArt should be restored")
+
+	// Should still have exactly 3 articles.
+	assert.Len(t, cat.Articles, 3)
+}
+
+func TestThreadedNewsYAML_PostArticle_rollback_with_parent(t *testing.T) {
+	// Test that FirstChildArt is restored when posting a reply.
+	n := &ThreadedNewsYAML{
+		ThreadedNews: hotline.ThreadedNews{
+			Categories: map[string]hotline.NewsCategoryListData15{
+				"TestCat": {
+					Type: hotline.NewsCategory,
+					Name: "TestCat",
+					Articles: map[uint32]*hotline.NewsArtData{
+						1: {
+							Title: "Parent Article",
+							// No FirstChildArt set — this is the first reply.
+						},
+					},
+					SubCats: make(map[string]hotline.NewsCategoryListData15),
+				},
+			},
+		},
+		filePath: "/nonexistent/dir/ThreadedNews.yaml",
+	}
+
+	newArticle := hotline.NewsArtData{
+		Title:  "Reply",
+		Poster: "tester",
+	}
+
+	err := n.PostArticle([]string{"TestCat"}, 1, newArticle)
+	assert.Error(t, err)
+
+	cat := n.ThreadedNews.Categories["TestCat"]
+
+	// FirstChildArt should be restored to zero.
+	assert.Equal(t, [4]byte{}, cat.Articles[1].FirstChildArt, "parent's FirstChildArt should be restored")
+
+	// New article should not exist.
+	_, exists := cat.Articles[2]
+	assert.False(t, exists, "reply article should be removed on write failure")
+}
+
+func TestThreadedNewsYAML_DeleteArticle_rollback(t *testing.T) {
+	n := &ThreadedNewsYAML{
+		ThreadedNews: hotline.ThreadedNews{
+			Categories: map[string]hotline.NewsCategoryListData15{
+				"TestCat": {
+					Type: hotline.NewsCategory,
+					Name: "TestCat",
+					Articles: map[uint32]*hotline.NewsArtData{
+						1: {
+							Title: "Article to delete",
+						},
+					},
+					SubCats: make(map[string]hotline.NewsCategoryListData15),
+				},
+			},
+		},
+		filePath: "/nonexistent/dir/ThreadedNews.yaml",
+	}
+
+	err := n.DeleteArticle([]string{"TestCat"}, 1, false)
+	assert.Error(t, err)
+
+	cat := n.ThreadedNews.Categories["TestCat"]
+
+	// Deleted article should be restored.
+	art, exists := cat.Articles[1]
+	assert.True(t, exists, "deleted article should be restored on write failure")
+	assert.Equal(t, "Article to delete", art.Title)
+}
diff --git a/internal/mobius/transaction_handlers.go b/internal/mobius/transaction_handlers.go
index ebf18e6..259ac02 100644
--- a/internal/mobius/transaction_handlers.go
+++ b/internal/mobius/transaction_handlers.go
@@ -84,9 +84,16 @@ const (
 	ErrMsgPermanentBan = "You are permanently banned on this server"
 
 	// General error messages
-	ErrMsgAccountNotFound = "Account not found."
-	ErrMsgUserNotFound    = "User not found."
-	ErrMsgCreateAlias     = "Error creating alias"
+	ErrMsgAccountNotFound      = "Account not found."
+	ErrMsgUserNotFound         = "User not found."
+	ErrMsgCreateAlias          = "Error creating alias"
+	ErrMsgUpdateAccount        = "Error updating account."
+	ErrMsgReadNewsCategories   = "Error reading news categories."
+	ErrMsgCreateNewsCategory   = "Error creating news category."
+	ErrMsgCreateNewsFolder     = "Error creating news folder."
+	ErrMsgDeleteNewsArticle    = "Error deleting news article."
+	ErrMsgPostNewsArticle      = "Error posting news article."
+	ErrMsgReadMessageBoard     = "Error reading message board."
 )
 
 // Converts bytes from Mac Roman encoding to UTF-8
@@ -621,6 +628,7 @@ func HandleSetUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotlin
 	err := cc.Server.AccountManager.Update(*account, account.Login)
 	if err != nil {
 		cc.Logger.Error("Error updating account", "Err", err)
+		return cc.NewErrReply(t, ErrMsgUpdateAccount)
 	}
 
 	// Notify connected clients logged in as the user of the new access level
@@ -1251,6 +1259,7 @@ func HandleGetNewsCatNameList(cc *hotline.ClientConn, t *hotline.Transaction) (r
 		b, err := io.ReadAll(&cat)
 		if err != nil {
 			cc.Logger.Error("get news categories", "err", err)
+			return cc.NewErrReply(t, ErrMsgReadNewsCategories)
 		}
 
 		fields = append(fields, hotline.NewField(hotline.FieldNewsCatListData15, b))
@@ -1282,6 +1291,7 @@ func HandleNewNewsCat(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot
 	err = cc.Server.ThreadedNewsMgr.CreateGrouping(pathStrs, name, hotline.NewsCategory)
 	if err != nil {
 		cc.Logger.Error("error creating news category", "err", err)
+		return cc.NewErrReply(t, ErrMsgCreateNewsCategory)
 	}
 
 	return []hotline.Transaction{cc.NewReply(t)}
@@ -1310,6 +1320,7 @@ func HandleNewNewsFldr(cc *hotline.ClientConn, t *hotline.Transaction) (res []ho
 	err = cc.Server.ThreadedNewsMgr.CreateGrouping(pathStrs, name, hotline.NewsBundle)
 	if err != nil {
 		cc.Logger.Error("error creating news bundle", "err", err)
+		return cc.NewErrReply(t, ErrMsgCreateNewsFolder)
 	}
 
 	return append(res, cc.NewReply(t))
@@ -1465,6 +1476,7 @@ func HandleDelNewsArt(cc *hotline.ClientConn, t *hotline.Transaction) (res []hot
 	err = cc.Server.ThreadedNewsMgr.DeleteArticle(pathStrs, uint32(articleID), deleteRecursive)
 	if err != nil {
 		cc.Logger.Error("error deleting news article", "err", err)
+		return cc.NewErrReply(t, ErrMsgDeleteNewsArticle)
 	}
 
 	return []hotline.Transaction{cc.NewReply(t)}
@@ -1512,6 +1524,7 @@ func HandlePostNewsArt(cc *hotline.ClientConn, t *hotline.Transaction) (res []ho
 	)
 	if err != nil {
 		cc.Logger.Error("error posting news article", "err", err)
+		return cc.NewErrReply(t, ErrMsgPostNewsArticle)
 	}
 
 	return append(res, cc.NewReply(t))
@@ -1533,6 +1546,7 @@ func HandleGetMsgs(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotlin
 	newsData, err := io.ReadAll(cc.Server.MessageBoard)
 	if err != nil {
 		cc.Logger.Error("Error reading messageboard", "err", err)
+		return cc.NewErrReply(t, ErrMsgReadMessageBoard)
 	}
 
 	return append(res, cc.NewReply(t, hotline.NewField(hotline.FieldData, newsData)))
diff --git a/internal/mobius/transaction_handlers_test.go b/internal/mobius/transaction_handlers_test.go
index f4699e3..9e995b5 100644
--- a/internal/mobius/transaction_handlers_test.go
+++ b/internal/mobius/transaction_handlers_test.go
@@ -1580,6 +1580,41 @@ func TestHandleGetMsgs(t *testing.T) {
 			},
 		},
 		{
+			name: "when ReadAll returns an error",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessNewsReadArt)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					Server: &hotline.Server{
+						MessageBoard: func() *mockReadWriteSeeker {
+							m := mockReadWriteSeeker{}
+							m.On("Seek", int64(0), 0).Return(int64(0), nil)
+							m.On("Read", mock.AnythingOfType("[]uint8")).Return(0, errors.New("read error"))
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranGetMsgs, [2]byte{0, 1},
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte("Error reading message board.")),
+					},
+				},
+			},
+		},
+		{
 			name: "when user does not have required permission",
 			args: args{
 				cc: &hotline.ClientConn{
@@ -2145,6 +2180,51 @@ func TestHandleDelNewsArt(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "when DeleteArticle returns an error",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessNewsDeleteArt)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					ID:     [2]byte{0, 1},
+					Server: &hotline.Server{
+						ThreadedNewsMgr: func() *hotline.MockThreadNewsMgr {
+							m := hotline.MockThreadNewsMgr{}
+							m.On("DeleteArticle", []string{"test"}, uint32(1), false).Return(errors.New("write error"))
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranDelNewsArt, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldNewsPath,
+						[]byte{
+							0, 1,
+							0, 0,
+							4,
+							0x74, 0x65, 0x73, 0x74,
+						},
+					),
+					hotline.NewField(hotline.FieldNewsArtID, []byte{0, 0, 0, 1}),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					ClientID:  [2]byte{0, 1},
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte("Error deleting news article.")),
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -3874,62 +3954,51 @@ func TestHandleNewNewsFldr(t *testing.T) {
 				},
 			},
 		},
-		//{
-		//	Name: "when there is an error writing the threaded news file",
-		//	args: args{
-		//		cc: &hotline.ClientConn{
-		//			Account: &hotline.Account{
-		//				Access: func() hotline.AccessBitmap {
-		//					var bits hotline.AccessBitmap
-		//					bits.Set(hotline.AccessNewsCreateFldr)
-		//					return bits
-		//				}(),
-		//			},
-		//			logger: NewTestLogger(),
-		//			Type:     [2]byte{0, 1},
-		//			Server: &hotline.Server{
-		//				ConfigDir: "/fakeConfigRoot",
-		//				FS: func() *hotline.MockFileStore {
-		//					mfs := &MockFileStore{}
-		//					mfs.On("WriteFile", "/fakeConfigRoot/ThreadedNews.yaml", mock.Anything, mock.Anything).Return(os.ErrNotExist)
-		//					return mfs
-		//				}(),
-		//				ThreadedNews: &ThreadedNews{Categories: map[string]NewsCategoryListData15{
-		//					"test": {
-		//						Type:     []byte{0, 2},
-		//						Count:    nil,
-		//						NameSize: 0,
-		//						Name:     "test",
-		//						SubCats:  make(map[string]NewsCategoryListData15),
-		//					},
-		//				}},
-		//			},
-		//		},
-		//		t: NewTransaction(
-		//			TranGetNewsArtNameList, [2]byte{0, 1},
-		//			NewField(hotline.FieldFileName, []byte("testFolder")),
-		//			NewField(hotline.FieldNewsPath,
-		//				[]byte{
-		//					0, 1,
-		//					0, 0,
-		//					4,
-		//					0x74, 0x65, 0x73, 0x74,
-		//				},
-		//			),
-		//		),
-		//	},
-		//	wantRes: []hotline.Transaction{
-		//		{
-		//			ClientID:  [2]byte{0, 1},
-		//			Flags:     0x00,
-		//			IsReply:   0x01,
-		//			Type:      [2]byte{0, 0},
-		//			ErrorCode: [4]byte{0, 0, 0, 1},
-		//			Fields: []hotline.Field{
-		//				NewField(hotline.FieldError, []byte("Error creating news folder.")),
-		//			},
-		//		},
-		//	},
+		{
+			name: "when CreateGrouping returns an error",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessNewsCreateFldr)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					ID:     [2]byte{0, 1},
+					Server: &hotline.Server{
+						ThreadedNewsMgr: func() *hotline.MockThreadNewsMgr {
+							m := hotline.MockThreadNewsMgr{}
+							m.On("CreateGrouping", []string{"test"}, "testFolder", hotline.NewsBundle).Return(errors.New("write error"))
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranGetNewsArtNameList, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldFileName, []byte("testFolder")),
+					hotline.NewField(hotline.FieldNewsPath,
+						[]byte{
+							0, 1,
+							0, 0,
+							4,
+							0x74, 0x65, 0x73, 0x74,
+						},
+					),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					ClientID:  [2]byte{0, 1},
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte("Error creating news folder.")),
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -4031,6 +4100,43 @@ func TestHandlePostNewsArt(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "when PostArticle returns an error",
+			args: args{
+				cc: &hotline.ClientConn{
+					Server: &hotline.Server{
+						ThreadedNewsMgr: func() *hotline.MockThreadNewsMgr {
+							m := hotline.MockThreadNewsMgr{}
+							m.On("PostArticle", []string{"www"}, uint32(0), mock.AnythingOfType("hotline.NewsArtData")).Return(errors.New("write error"))
+							return &m
+						}(),
+					},
+					Logger: NewTestLogger(),
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessNewsPostArt)
+							return bits
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranPostNewsArt,
+					[2]byte{0, 0},
+					hotline.NewField(hotline.FieldNewsPath, []byte{0x00, 0x01, 0x00, 0x00, 0x03, 0x77, 0x77, 0x77}),
+					hotline.NewField(hotline.FieldNewsArtID, []byte{0x00, 0x00, 0x00, 0x00}),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte("Error posting news article.")),
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -4611,3 +4717,268 @@ func TestHandleInviteToChat(t *testing.T) {
 		})
 	}
 }
+
+func TestHandleSetUser(t *testing.T) {
+	type args struct {
+		cc *hotline.ClientConn
+		t  hotline.Transaction
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantRes []hotline.Transaction
+	}{
+		{
+			name: "when user does not have required permission",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							return bits
+						}(),
+					},
+					Server: &hotline.Server{},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranSetUser, [2]byte{0, 1},
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					Flags:     0x00,
+					IsReply:   0x01,
+					Type:      [2]byte{0, 0},
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte(ErrMsgNotAllowedModifyAccounts)),
+					},
+				},
+			},
+		},
+		{
+			name: "when account is not found",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessModifyUser)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					ID:     [2]byte{0, 1},
+					Server: &hotline.Server{
+						AccountManager: func() *MockAccountManager {
+							m := MockAccountManager{}
+							m.On("Get", "testuser").Return((*hotline.Account)(nil))
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranSetUser, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldUserLogin, hotline.EncodeString([]byte("testuser"))),
+					hotline.NewField(hotline.FieldUserName, []byte("Test User")),
+					hotline.NewField(hotline.FieldUserAccess, make([]byte, 8)),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					ClientID:  [2]byte{0, 1},
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte(ErrMsgAccountNotFound)),
+					},
+				},
+			},
+		},
+		{
+			name: "when Update returns an error",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessModifyUser)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					ID:     [2]byte{0, 1},
+					Server: &hotline.Server{
+						AccountManager: func() *MockAccountManager {
+							m := MockAccountManager{}
+							m.On("Get", "testuser").Return(&hotline.Account{
+								Login: "testuser",
+								Name:  "Old Name",
+							})
+							m.On("Update", mock.Anything, "testuser").Return(errors.New("disk full"))
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranSetUser, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldUserLogin, hotline.EncodeString([]byte("testuser"))),
+					hotline.NewField(hotline.FieldUserName, []byte("New Name")),
+					hotline.NewField(hotline.FieldUserAccess, make([]byte, 8)),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					ClientID:  [2]byte{0, 1},
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte(ErrMsgUpdateAccount)),
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotRes := HandleSetUser(tt.args.cc, &tt.args.t)
+
+			TranAssertEqual(t, tt.wantRes, gotRes)
+		})
+	}
+}
+
+func TestHandleNewNewsCat(t *testing.T) {
+	type args struct {
+		cc *hotline.ClientConn
+		t  hotline.Transaction
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantRes []hotline.Transaction
+	}{
+		{
+			name: "when user does not have required permission",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							return bits
+						}(),
+					},
+					Server: &hotline.Server{},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranNewNewsCat, [2]byte{0, 1},
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					Flags:     0x00,
+					IsReply:   0x01,
+					Type:      [2]byte{0, 0},
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte(ErrMsgNotAllowedCreateNewsCategories)),
+					},
+				},
+			},
+		},
+		{
+			name: "when CreateGrouping returns an error",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessNewsCreateCat)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					ID:     [2]byte{0, 1},
+					Server: &hotline.Server{
+						ThreadedNewsMgr: func() *hotline.MockThreadNewsMgr {
+							m := hotline.MockThreadNewsMgr{}
+							m.On("CreateGrouping", []string{"test"}, "TestCat", hotline.NewsCategory).Return(errors.New("write error"))
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranNewNewsCat, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldNewsCatName, []byte("TestCat")),
+					hotline.NewField(hotline.FieldNewsPath,
+						[]byte{
+							0, 1,
+							0, 0,
+							4,
+							0x74, 0x65, 0x73, 0x74,
+						},
+					),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					ClientID:  [2]byte{0, 1},
+					IsReply:   0x01,
+					ErrorCode: [4]byte{0, 0, 0, 1},
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldError, []byte(ErrMsgCreateNewsCategory)),
+					},
+				},
+			},
+		},
+		{
+			name: "with a valid request",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessNewsCreateCat)
+							return bits
+						}(),
+					},
+					Logger: NewTestLogger(),
+					ID:     [2]byte{0, 1},
+					Server: &hotline.Server{
+						ThreadedNewsMgr: func() *hotline.MockThreadNewsMgr {
+							m := hotline.MockThreadNewsMgr{}
+							m.On("CreateGrouping", []string{"test"}, "TestCat", hotline.NewsCategory).Return(nil)
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranNewNewsCat, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldNewsCatName, []byte("TestCat")),
+					hotline.NewField(hotline.FieldNewsPath,
+						[]byte{
+							0, 1,
+							0, 0,
+							4,
+							0x74, 0x65, 0x73, 0x74,
+						},
+					),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					ClientID: [2]byte{0, 1},
+					IsReply:  0x01,
+					Fields:   []hotline.Field{},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotRes := HandleNewNewsCat(tt.args.cc, &tt.args.t)
+
+			TranAssertEqual(t, tt.wantRes, gotRes)
+		})
+	}
+}